-
Notifications
You must be signed in to change notification settings - Fork 236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle no-topic topic ("general chat") #1250
Comments
We'll use this to make a type-level distinction between a string that's being used to name a topic, and just any old string. That will help us in particular with zulip#1250 the "general chat" feature, where the way we show a topic name to the user stops being necessarily just the same string that it appears as in the API. The next phase will be to migrate a bunch of places in our code to refer to TopicName when that's what they mean, instead of just String. During this phase, a TopicName can be freely used as a String, but not vice versa. So we'll do the migrations mostly in order of the data flow, from upstream to downstream. That will allow us to do them over a series of individually coherent commits, with a minimum of occasions where we temporarily introduce a conversion that won't be needed in the final result. That means: first, test data; then topics returned from the API to our code; then our internal models; then back to the API, this time for topics passed to the API from our code. After we have the type in place all over where it belongs, we'll start making use of the distinction, and then enforcing it.
Each of these call sites will need to be updated for zulip#1250. Those sites can now be found by listing the references to this getter. As a bonus the type-checker will point out most of the places that need updates, because this getter will become nullable. (The exceptions are where it's used in a string interpolation, because `null.toString()` is valid and returns "null".)
We're now explicitly using apiName, canonicalize, or displayName in each of the places where we had been implicitly treating a topic name as a String. Let the type-checker catch any future regressions on that front, by removing "implements String". More precisely, I *think* we've covered each such place. The loophole is that Object methods can still be called, including toString, so an interpolation like "${channelName} > ${topic}" won't be flagged by the analyzer. I found some such interpolations with a grep, but that's necessarily heuristic and there could be more. So the analyzer won't give us quite as much help here as we'd like; but it'll give quite a bit, and this is the most I see how to do. Without the "implements String", a TopicName value can no longer be implicitly converted to String or have String methods called on it. Instead the type-checker will require any code that has such a value to call one of the members we've declared on TopicName (or a member of Object) in order to do anything with it. That way the code will be explicit about whether it needs the API name, or the display name, or the canonicalized form for making equality comparisons. And that in turn will enable us to make "display name" behave differently from "API name", for zulip#1250 the "general chat" feature, with a reliable way of tracking down which sites need which version.
I've just sent #1266 as a refactor that will set us up to implement this, by distinguishing a TopicName type from plain String. Other than that PR, I'd like to not drive this feature myself, in order to focus on other issues. After that refactor, the places that will need to adapt to handle the empty topic differently from other topics can be found as the references to [TopicName.displayName]. There are about 9 such places. A couple of them will probably raise questions that should be discussed in the #design channel. I believe the other pieces needed for the implementation are
|
The server will start allowing channel messages to have an empty topic at the API level, replacing the old "(no topic)" behavior. This should be displayed as something like "general chat" — with italics — in the UI.
The exact phrase to use for "general chat" is specified by the server in the API. For compatibility with old clients, the feature is gated by a flag the client sends. For details, see references below.
This is a feature planned for the upcoming Zulip Server 10.
Plan
We'll want a version of this soon, as work is proceeding on the server side. This will be a task for a member of the core team.
References
The text was updated successfully, but these errors were encountered: