We add these two functions to the API,
so that we no longer have `alert_words_ui`
using private data from `alert_word`:
alert_words.has_alert_word()
alert_words.get_word_list()
And to initialize the data, we have a proper
`initialize` method that is passed in only
the parameters that it needs from `ui_init`.
(We also move the step of deleting `alert_words`
from `page_params` to the `ui_init` module.)
Because it's a bit less cumbersome to initialize
`alert_words`, we now just it directly in the
node tests for `alert_words_ui`.
This is follow up to da79fd206a
I accidentally skipped over pm_conversations. Same
ideas as the bigger previous commit--we pass in params
to the initialize function and do the delete cleanup
within ui_init.
This cleans up the handoff of page_params
data between ui_init and modules that
take over ownership of page_params-derived
data.
Read the long comment in ui_init for a bit
more context.
Most of this diff is actually test cleanup.
And a lot of the diff to "real" code is
just glorified `s/page_params/params/`
in the `initialize` functions.
One little oddity is that we don't actually
surrender ownership of `page_params.user_id`
to `people.js`. We could plausibly sweep
the rest of the codebase to just use
`people.my_user_id()` consistently, but it's
not a super high priority thing to fix,
since the value never changes.
The stream_data situation is a bit messy,
since we consume `page_params` data in the
initialize() function in addition to the
`params` data we "own". I added a comment
there and intend to follow up. I tried
to mostly avoid the "word soup" by extracting
three locals at the top.
Finally, I don't touch `alert_words` yet,
despite it also doing the delete-page-params-data
dance. The problem is that `alert_words`
doesn't have a proper `initialize()`. We
should clean that up and have it use a
`Map` internally, too.
This gives them cache-compatible URLs, and also avoids some extra
copies of the sprite sheet images.
Comments on the Octopus emoji added by tabbott.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This is not always a behavior-preserving translation: _.defaults
mutates its first argument. However, the code does not always appear
to have been written to expect that.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This is not always a behavior-preserving translation: _.extend mutates
its first argument. However, the code does not always appear to have
been written to expect that.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This moves some code from settings_display.js
into the new module settings_config.js.
Extracting this module breaks some dependencies
on settings_display.js (which has some annoying
transitive dependencies, including jQuery).
In particular this isolates stream_data from
from settings_display.js.
Two of the three structures that we moved here
weren't even directly used by settings_display.js,
since we do a lot of rendering in the modules
admin.js and setting.js.
We make get_all_display_settings() a function
to avoid a require-time dependency on page_params.
Breaking the dependencies simplifies a few
node tests.
Most of the node test complexity came from the
following commit in March 2019:
5a130097bf
The commit itself seems harmless enough, but
dependencies can have a somewhat "viral" nature,
where making stream_data depend on settings_display
caused us to modify four different node tests.
This refactoring is the first step toward sharing
our markdown code with mobile. This focuses on
the Zulip layer, not the underlying third party `marked`
library.
In this commit we do a one-time initialization to
wire up the markdown functions, but after further
discussions with Greg, it might make more sense
to just pass in helpers on every use of markdown
(which is generally only once per sent message).
I'll address that in follow-up commits.
Even though it looks like a pretty invasive change,
you will note that we barely needed to modify the
node tests to make this pass. And we have pretty
decent test coverage here.
All of the places where we used to depend on
other Zulip modules now use helper functions that
any client (e.g. mobile) can configure themselves.
Or course, in the webapp, we configure these from
modules like people/stream_data/hash_util/etc.
Even in places where markdown used to deal directly with
data structures from other modules, we now use functions.
We may revisit this in a future commit, and we might
just pass data directly for certain things.
I decided to keep the helpers data structure completely flat,
so we don't have ugly nested names like
`helpers.emoji.get_emoji_codepoint`. Because of this,
some of the names aren't 1:1, which I think is fine.
For example, we map `user_groups.is_member_of` to
`is_member_of_user_group`.
It's likely that mobile already has different names
for their versions of these functions, so trying for
fake consistency would only help the webapp. In some
cases, I think the webapp functions have names that
could be improved, but we can clean that up in future
commits, and since the names aren't coupled to markdown
itself (i.e. only the config), we will be less
constrained.
It's worth noting that `marked` has an `options`
data structure that it uses for configuration, but
I didn't piggyback onto it, since the `marked`
options are more at the lexing/parsing layer vs.
the app-data layer stuff that our helpers mostly
help with.
Hopefully it's obvious why I just put helpers in
the top-level namespace for the module rather than
passing it around through multiple layers of the
parser.
There were a couple places in markdown where we
were doing awkward `hasOwnProperty` checks for
emoji-related stuff. Now we use the Python
principle of ask-forgiveness-not-permission and
just handle the getters returning falsy data. (It
should be `undefined`, but any falsy value is
unworkable in the places I changed, so I use
the simpler, less brittle form.)
We also break our direct dependency on
`emoji_codes.json` (with some help from the
prior commit).
In one place I rename streamName to stream_name,
fixing up an ancient naming violation that goes
way back to before this code was even extracted
away from echo.js. I didn't bother to split this
out into a separate commit, since 2 of the 4
lines would be immediately re-modified in the
subsequent commit.
Note that we still depend on `fenced_code`
via the global namespace, instead of simply
requiring it directly or injecting it. The
reason I'm postponing any action there is that
we'll have to change things once we move
markdown into a shared library. (The most
likely outcome is that we'll rename/move both files
at the same time and fix the namespace/require
details as part of that commit.)
Also the markdown code still relies on `_` being
available in the global namespace. We aren't
quite ready to share code with mobile yet, but the
underscore dependency should not be problematic,
since mobile already uses underscore to use the
webapp's shared typing_status module.
This mostly moves logic into people.js.
The people functions added here are glorified
two-liners.
One thing that changes here is that we
are a bit more rigorous about duplicate
names.
The code is slightly awkward, because this
commit preserves the strange behavior
that if 'alice|42' doesn't match on
the user with the name "alice" and user_id
"42", we instead look for a user whose
name is "alice|42". That seems like a
misfeature to me, but there's a test for
it, so I want to check with Tim that it's not
intentional behavior before I simplify
the code.
We add this API to emoji.js, so that markdown
doesn't need to look at internal data structures
(or even need to understand any kind of record
format for results).
Here are the functions:
get_realm_emoji_url()
get_emoji_name()
get_emoji_codepoint()
We use the API now in markdown, which eliminates
the need for the markdown parser to require
the emoji JSON file.
Each function has a simple docstring:
get_emoji_name('1f384') === 'holiday_tree'
get_emoji_codepoint('avocado') === '1f951'
get_realm_emoji_url('shrug') === '/user_avatars/2/emoji/images/31.png'
Also we have simple test coverage for the API
(including tests that verify the docstrings).
This name was misleading, because we weren't
actually setting realm_filters (that's what
`page_params.realm_filters = realm_filters`
is for); we were instead updating our
realm filter rules.
These functions were just shims that were
used in the somewhat painful migration from
subject_* to topic_*.
The commit 4572be8c27
fixed it so that the client never needs to
deal with "subject_links".
So now we just go back to simpler code:
message.topic_links = links
links = message.topic_links
I am not quite ready to declare victory on
the subject/topic migration, but we are super
close. In this commit I bump a blueslip
warning to a blueslip error, so that we'll
be notified of any codepath that is still
using the janky fall-back-to-subject defensive
code here.
If we go a couple days without any errors, then
we can remove the blueslip warning and the
defensive code immediately and then inline
the callers at our leisure. I wouldn't be
wildly against keeping these wrappers in some
parts of the code, but that debate is out of
the scope of this immediate fix, and I haven't
thought hard about it yet.
We can basically sweep set_message_topic() now,
if we wanted to, since it's truly just a one-liner.
(At one point it was encapsulating something
like `message.subject = foo`).
This required a tiny change to compose_fade
test setup.
Most of this logic is specific to markdown
message processing, so we move the code to
markdown.js.
The only responsibility that we leave with
`emoji.js` is to provide us with a list
of translations (regex and replacement text).
But now `markdown.js` actually (directly) executes
those translations against Zulip messages
as part of its preprocessing.
This should simplify the upcoming mobile conversion.
Instead of mobile needing to duplicate this fairly
complex function, they will just need to pass
us in a list similar to `emoji_translations` inside
of `emoji.js`. That code has a comment that shows
what the data structure looks like.
I am 99% sure we can rely on trimRight() and
trim() being available in all browsers that
we support. I verified in FF.
This removes the util dependency from both
modules touched here.
We now treat util like a leaf module and
use "require" to import it everywhere it's used.
An earlier version of this commit moved
util into our "shared" library, but we
decided to wait on that. Once we're ready
to do that, we should only need to do a
simple search/replace on various
require/zrequire statements plus a small
tweak to one of the custom linter checks.
It turns out we don't really need util.js
for our most immediate code-sharing goal,
which is to reuse our markdown code on
mobile. There's a little bit of cleanup
still remaining to break the dependency,
but it's minor.
The util module still calls the global
blueslip module in one place, but that
code is about to be removed in the next
few commits.
I am pretty confident that once we start
sharing things like the typeahead code
more aggressively, we'll start having
dependencies on util. The module is barely
more than 300 lines long, so we'll probably
just move the whole thing into shared
rather than break it apart. Also, we
can continue to nibble away at the
cruftier parts of the module.
We used to have a block of code doing this just in the presence
endpoint because that's where we'd had error-handling problems with it
not being present, but it seems more correct for it to run
unconditionally on all HTTP requests.
This requires adding a dependency of channel on reload_state, which we
record in the webpack configuration for now.
