zulip-terminal
zulip-terminal copied to clipboard
zulip
[WIP] Unify case sensitive topic names
This unifies case sensitive topic names using lowercase topic names as an invariant.
Crux
We need some kind of invariant which we can rely upon for lookups and comparisons given that topics can change their casing at any point. Consequently, I propose to use lowercase topics as keys wherever store/index data.
Commits
The current commit structure is temporary. I have fixed one thing per commit to represent how I went about the changes but we would definitely want to squash them (except the first) before merging.
I would greatly appreciate feedback about the proposal and what else should be fixed.
@neiljp Thanks for the review and the pointers! :+1:
I have reworked the fundamental approach that I had to now store lowercase topic names for lookups and comparisons (see https://github.com/zulip/zulip-terminal/pull/733#issue-455669765). I have also addressed the three issues that you reported.
Updated with improved commits, more comments and test amendments (except which are related to muted topics).
@preetmishra This looks to cover a good number of comparison cases; this was blocked on another PR?
To clarify, this uses:
- lower case in internal structures (that seems clearest)
- "latest" topic names in display? (last-but-one commit references that?)
- the 'real' case in what remaining places? (for display ^)
I think this will be clearer with the now-merged #675 and when the topic list updates with something like #785, as we should be able to test internally more easily.
This looks good, though I've not dug into all the cases so far. Pending further review, this seems reasonable - my concern is whether we might consider locally handling topics with ids, which may simplify this issue - ie. each topic_id in a stream and (stream_id, topic_id) would be unique, and so we can have a 'latest name' (for display) for each id, and the comparisons would all occur at the point where topic names are converted to ids.
The first commit seems like a separate cleanup, is that correct?
Heads up @preetmishra, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.
