I have looked at maybe ~100 errors in the last week as part
of fixing the tooling, and it's quite common to want to just
see what the improved file would look like. Now I show the
desired output with line numbers.
I also try to encourage devs to scroll up, since newbies
often don't do that for some reason when confronted with
error output.
Finally, I add some color. I try to repeat myself without
color for certain things in case colors on certain
backgrounds are hard to read.
A fast way to test this is to just break up a long tag
into two lines.
`Press Enter to send` used to hide `Send` button, we remove that
behaviour.
We show the current state of `Enter` hotkey action via text below
`Send` button which can toggle behaviour on click.
get_object_from_key should be used when trying to fetch a Confirmation
object. There are some places that need to make
Confirmation.objects.filter(...) queries, so we can't completely ban the
pattern, but we can ban .get(...) and
.filter(..., confirmation_key=..., ...).
Now we only tokenize the file once, and we pass
**validated** tokens to the pretty printer.
There are a few reasons for this:
* It obviously saves a lot of extra computation
just in terms of tokenization.
* It allows our validator to add fields
to the Token objects that help the pretty
printer.
I also removed/tweaked a lot of legacy tests for
pretty_print.py that were exercising bizarrely
formatted HTML that we now simply ban during the
validation phase.
This accomplishes a few things:
* lighten the load for the main validation loop
* defer indentation checks until we are sure the author
even knows how to match up tags
* add some info to the Token objects that we may soon
consume in our pretty-printer
We now complain about programmers who don't use
4-space indents in template files, rather than
letting the pretty printer fix them.
This is partly just to simplify the pretty printer
code (in future commits), but it also makes the
symptom more obvious to newbie developers. They
are probably just as able to react to the direct
error messages as they are able to figure out how
to read diffs from the pretty printer and grok
the --fix syntax. And once they learn the convention
and configure their editor, it should then be a
one time problem.
We now create tokens for whitespace and text, such that you
could rebuild the template file with "".join(token.s for
token in tokens).
I also fixed a few bugs related to not parsing
whitespace-control tokens.
We no longer ignore template variables, although we could do
a lot better at validating them.
The most immediate use case for the more thorough parser is
to simplify the pretty printer, but it should also make it
less likely for us to skip over new template constructs
(i.e. the tool will fail hard rather than acting strange).
Note that this speeds up the tool by almost 3x, which may be
slightly surprising considering we are building more tokens.
The reason is that we are now munching efficiently through
big chunks of whitespace and text at a time, rather than
checking each individual character to see if it starts one
of the N other token types.
The changes to the pretty_print module here are a bit ugly,
but they should mostly be made irrelevant in subsequent
commits.
This tool has been unmaintained since our initial code
sweep to fix templates, and it has possibly bit-rotted
during unrelated code sweeps like introducing mypy, etc.
It's not documented anywhere.
The preferred method now is to run:
./tools/check-templates --fix
String 'Here are a few messages I understand:'(next commit) was failing
./tools/check-capitalization check because of the capital I. I added
'I understand' to the IGNORED_PHRASES list in tools/lib/capitalization.py.
Adding "I" was working as well but didn't seem to me as a very great fix.
Strangely enough, adding " I " to the list made the test fail again
(With a lot of failed strings this time) as mentioned in the following
CZO thread.
Relevent CZO chat -
https://chat.zulip.org/#narrow/stream/49-development-help/topic/capitalization.20confusion.2E
In https://github.com/jorisroovers/gitlint/pull/246 I split the
gitlint package into gitlint and gitlint-core, where the latter avoids
pinning exact versions of its requirements so we can use it again.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The main focus is on improving the instructions around claiming issues
to try to create less issue claiming spam.
Additionally, we haven't done a detailed update in a few years, and
some of the content is stale/irrelevant.
I rewrote most of tools/lib/pretty-printer.py, which
was fairly easy due to being able to crib some
important details from the previous implementation.
The main motivation for the rewrite was that we weren't
handling else/elif blocks correctly, and it was difficult
to modify the previous code. The else/elif shortcomings
were somewhat historical in nature--the original parser
didn't recognize them (since they weren't in any Zulip
templates at the time), and then the pretty printer was
mostly able to hack around that due to the "nudge"
strategy. Eventually the nudge strategy became too
brittle.
The "nudge" strategy was that we would mostly trust
the existing templates, and we would just nudge over
some lines in cases of obviously faulty indentation.
Now we are bit more opinionated and rigorous, and
we basically set the indentation explicitly for any
line that is not in a code/script block. This leads
to this diff touching several templates for mostly
minor fix-ups.
We aren't completely opinionated, as we respect the
author's line wrapping decisions in many cases, and
we also allow authors not to indent blocks within
the template language's block constructs.
In cases where an opening tag is so long that we stretch
it to 2+ lines of code, we should try to use block-style
formatting in the template code.
Unfortunately, we have lots of legacy code that violates
this concept, so this is a timid fix.
There are also legit use cases like textarea where we
probably need to keep the ugly template syntax for things
to render properly.
We disallow this HTML:
junk-text-before-open-tag<p>
This is a paragraph.
</p>
We rarely see the above mistake, but we want to eliminate
the possibility to be somewhat rigorous, and so that we
can eliminate a pretty-printer mis-feature.
Previously, running `./tools/run-dev.py` when provision was required
would lead to a warning along the lines of:
```
Before we run tests, we make sure your provisioning version
is correct by looking at var/provision_version, which is at
version 165.1, and we compare it to the version in source
control (version.py), which is 165.2.
It looks like you checked out a branch that has added
dependencies beyond what you last provisioned. Your command
is likely to fail until you add dependencies by provisioning.
Do this: `./tools/provision`
If you really know what you are doing, use --skip-provision-check to
run anyway.
```
The assumption that we're trying to run tests might cause some
confusion, especially if its the first time you're seeing the
provision warning. Hence, we reword the first paragraph to avoid
making that assumption.
The second paragraph has also been slightly altered, since (1) it's
possible that we didn't checkout a different branch, but eg just
rebased with upstream and (2) we might not be on a VM.
The warning you'd get after this commit would be along the lines of:
```
Provisioning state check failed! This check compares
`var/provision_version` (currently 165.2) to the version in
source control (`version.py`), which is 164.6, to see if you
likely need to provision before this command can run
properly.
The branch you are currently on expects an older version of
dependencies than the version you provisioned last. This may
be ok, but it's likely that you either want to rebase your
branch on top of upstream/main or re-provision your machine.
Do this: `./tools/provision`
If you really know what you are doing, use --skip-provision-check to
run anyway.
```
or along the lines of:
```
Provisioning state check failed! This check compares
`var/provision_version` (currently 165.2) to the version in
source control (`version.py`), which is 167.2, to see if you
likely need to provision before this command can run
properly.
The branch you are currently on has added dependencies beyond
what you last provisioned. Your command is likely to fail
until you add dependencies by provisioning.
Do this: `./tools/provision`
If you really know what you are doing, use --skip-provision-check to
run anyway.
```
The `current_queue_size` key in the queue monitoring stats file was
the local queue size, not the global queue size -- d5a6b0f99a
renamed the function, but did not adjust the queue monitoring JSON,
despite the last use of it having been removed in cd9b194d88.
The function is still used to mark "we emptied our queue," and it
remains a reasonable metric for that.
It's better for this to catch all exit(...) calls with non-zero exit
code, given the purpose is to catch all exits with failure, as opposed
to only exit(1).
Unhandled exceptions propagating to process_queue were not caught there,
causing improper logging - errors didn't land in errors.log as expected.
Exceptions should be caught and explicitly logged by the process_queue
logger. Exceptions occurring during consuming events are caught and
handled inside the worker's logic - however those that happen while
setting up the worker were not addressed at all, and that's the core bug
we mean to address here.
Furthermore, in multi-threaded mode we want the autoreload mechanism to
be working - which it doesn't without catching the exceptions. The
correct approach is to - again - catch the exception, log it and then
send SIGUSR1 signal to trigger exit and autoreload.
Unless this was working around a specific bug that has somehow
persisted for eight years, this was just pointlessly defeating
Python’s bytecode cache every time you started the dev server.
This reverts commits deffda072f,
59228f7458, and
ae45217671.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This tool helps catch common typos in code and documentation, which is
particularly useful for our many contributors who are not native
English speakers.
The config is based on the codespell that I ran in
https://github.com/zulip/zulip/pull/18535.
CircleCI expected 3434; GitHub Actions expects 1001. This is the
reason zulip-ci.yml currently needs to mess with the permissions in
/__w.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This will be useful to let users enable/disable
sharing read receipts once we add that feature.
Note: Added "I've" to IGNORED_PHRASES in
tools/lib/capitalization.py to avoid capitalization
errors for the label text of this setting.
We make zero invalid value for message_content_delete_limit_seconds and
for handling the case of "Allow to delete message any time", the API-level
value of message_content_delete_limit_seconds is "anytime" and "None"
as the DB-level value. We also use these values for message retention
setting, so it helps maintain consistency.
On a 2 GiB, 1 CPU system, webpack would hit the Node.js heap
limit (which is half of physical memory up to 4 GiB, on 64-bit
systems).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
gitlint has a bunch of pinned requirements that hold back important
upgrades and conflict with other packages’ requirements. The gitlint
author has rejected proposals to unpin them because it might increase
the amount of maintenance he needs to do
(https://github.com/jorisroovers/gitlint/pull/133). That decision is
his to make, but _somebody_ needs to do the maintenance, so we
delegate it to Debian and Ubuntu. If that means using a significantly
older version of gitlint, that’s a tradeoff we need to make to keep
the rest of our requirements current.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The semgrep --dangerously-allow-arbitrary-code-execution-from-rules
flag is deprecated and no longer used.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit adds a new module settings_defaults.js which calls
the functions in settings_display passing appropriate container
element and settings object as parameters.
We also add one more parameter for_realm_settings to some of the
functions in settings_dislay to differentiate between the user
and realm-level settings.
When the upstream provides a chunked response, proxying this header
causes a protocol-level miscommunication.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This environment variable is not a thing and has never been a thing,
while the path it purportedly pointed to does not exist and has never
existed. It appears to have been inexplicably both cargo-culted and
renamed from test-js-with-casper.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We will use this modal for any narrow / hash or other UI element that
requires an actual account to use, to provide something reasonable to
occur when a user clicks on those things.
This adds the X-Smokescreen-Role header to proxy connections, to track
usage from various codepaths, and enforces a timeout. Timeouts were
kept consistent with their previous values, or set to 5s if they had
none previously.
This was originally meant to fix the emoji mapping conflict during a
Slack import. In Slack, 🎉 and ㊗️ have different
symbols, but they both map to 🎉 in Zulip prior to this commit.
㊗️ now refers to the Japanese character version, as is
observed in Matrix and Slack.
I expand the fix to include all other Japanese characters. Matrix.org
and Slack already have those characters in their symbol section, and so
this is to reach feature parity.
See the discussion thread in https://chat.zulip.org/#narrow/stream/9-issues/topic/duplicate.20emoji.20in.20data.20import
These changes are all independent of each other; I just didn’t feel
like making dozens of commits for them.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
None results in an uninitialized image (that happens to be transparent
most of the time); we want to explicitly initialize the image to
transparent.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We rework the landing page for companies in the same way we've
recently revamped the landing pages for other use cases.
This implementation unfortunately duplicates a lot of content from
/plans; we should clean that up at some point.
We recently added a lot of new pages to our top navigation and
restructured top-navigation in general. This commit updates the
footer to reflect the recent changes to our top navigation.
Partially reverts commit cc55393671.
mode="r" cannot be combined with buffering=0.
This is not a correct thing to do, since a multibyte character might
get split between two binary reads and cause UnicodeDecodeError, but
it’s good enough for a known-broken test we don’t run.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This removes some steps which are no longer necessary to be run
in the production upgrade script. The steps were used due to
errors related to supervisor failing to restart which was resolved
in the commit 08c39a7388.
This removes a bunch of non-functional duplicate JavaScript, HTML, and
CSS that was interfering with maintenance on the functional originals,
because it was never clear how to update the duplicates or how to
check that you’d updated the duplicates correctly.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This API change removes unnecessary complexity from a client that
wants to change a user's personal settings, and also saves developers
from needing to make decisions about what sort of setting something is
at the API level.
We preserve the old settings endpoints as mapping to the same function
as the new one for backwards-compatibility. We delete the
documentation for the old endpoints, though the documentation for the
merged /settings endpoint mentions how to use the old endpoints when
needed.
We migrate all backend tests to the new endpoints, except for
individual tests for each legacy endpoint to verify they still work.
Co-authored-by: sahil839 <sahilbatra839@gmail.com>
We move templates related to listing streams in left pannel of
stream settings overlay to stream_settings folder by making
following changes:
* Rename `subscriptions.hbs` to `browse_streams_list.hbs`.
* Move `settings_stream_list.hbs` to stream_settings folder.
* Rename `subscription.hbs` to `browse_streams_list_item.hbs`.
* Move `settings_stream_list_item.hbs` to stream_settings folder.
This makes several changes:
* Fixes a bug where the help text explaining our policies was not displayed.
* No help text was defined for many organization types.
* Copy-edits the help text somewhat.
* Offers all of the organization type options.
* Removes the 100% coverage requirement because it's annoying to test
the e.currentTarget click handler.
This commit changes the bot-edit modal to use dialog_widget instead of
edit_fields_modal.
This commit also removes edit_fields_modal module as it is no longer used.
This commit adds a new dialog_widget.js file containing most
of the code of confirm_dialog.js with some minor changes and
changes confirm_dialog to be a wrapper around dialog_widget.js.
We pass 'is_confim_dialog' as true in dialog_widget for a
confirm_dialog modal. This commit also renames confirm_dialog.hbs
and confirm_dialog_heading.hbs to dialog_widget.js,
dialog_widget.hbs and dialog_widget_heading.hbs respectively.
We currently configure ‘APT::Get::Assume-Yes’ in our custom Docker
image, but this is the only place we rely on it (outside of the
Dockerfile itself), and it’s better not to.
Also ‘apt-get remove && apt-get purge’ is the same as just ‘apt-get
purge’.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit adds a check to avoid the use of assertTrue
for cases like: assertTrue(len(data) == 2).
We should use assert_length, assertGreater, or
assertGreaterEqual, whatever suits, in cases like these.
We use subs as a common variable name for a collection of stream
data structure used in settings, in lot of modules. So this
rename clears a bunch of related shadowed variables.
This commit migrates the `navbar.html` Django template
to handlebars by creating a new file as `navbar.hbs`
within `/static/templates` which is then rendered
using `ui_init` module.
As a part of migration, we also remove the `search_pills_enabled`
and `embedded` parameters from the context attribute as they
are no longer needed now.
Fixes part of #18792.
Now that we are starting to link this pages from the landing page's
top navigation, it makes sense to have proper backlinks to the
homepage so that there is some continuity when the user clicks on
a link that takes them to a ReadTheDocs page from the main website.
This commit first moves the compose.validate() function out
with the functions that are needed by it. Then one by one
checked for which function is now not needed in compose.js.
This moves all validation related functions out of "compose.js"
to "compose_validate.js".
Splitting compose announce variables out of compose.js.
This commit moves the "user_acknowledged_all_everyone" and
"user_acknowledged_announce" out of compose.js to reduce
cyclic dependency of compose_validate on compose.js.
Moving wildcard mentions to compose_validate.
The wildcard mention settings are mostly used while validating.
Also to reduce the cyclic dependence of compose in
compose_validate, the related wildcard mentions are moved out to
compose_vaidate.js.
This also converts reset_acknowledged functions to set values
by passing values.
Mypy can’t follow absolute imports based on directories other than the
root. This was hiding some type errors due to ignore_missing_imports.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This is consistent with how we handle JsonableError and friends; it
doesn't make sense for translators to spend time on strings only
visible in a development environment.
Even though this looks like an independently runnable script, it
should not be run independently: a SHA-256 mismatch will fail to stop
the script, unless it was sourced from another script that has ‘set
-e’.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commits ports the `keyboard_shortcuts.html` file from
using the Django template to handlebars, essentially creating
a new file as `keyboard_shortcuts.hbs` within /static/templates
which is then rendered using info_overlays.js.
Fixes part of #18792.
We create a new widget edit_fields_modal such that this common
framework can be used in bot-edit modal, linkifier-edit modal
and user-edit modal, which have very similar implementations.
The "edit-fields-modal-status" is used only for edit-linkifier
modal and remains empty for others, so this change does not
cause problems with other modals.
We had a lot of functions and click handlers that were only
involved with user profile modal and were not related to
popovers logic in any way. So we extract these functions
into a separate module `user_profile.js`.
We turn off the eslint no-use-before-define for TypeScript files
because it does not work correctly for types. There is already
a typescript-eslint version of it that is enabled for TS.
We also update the error handler on window to use instanceof check
for ErrorEvent instead of checking the error property.
The plan for type annotating the page_params is to set it to
Record<string, unknown> for now and then annotate individual
properties on it as we use it in typescript modules.
We add a exclude pattern that makes sure we don't catch two edge
cases: a variable declaration `const style =` and setting a
variable ending in style such as `require_cmd_style =`. We don't add
and exclude pattern for let declaration because it will catch lines
that modify it later in the code.
(Removed other files from exclude list that no longer needed to be
excluded from this lint rule.)
Moved `subscription_invites_warning` modal to `confirm_dialog`
folder and renamed the modal to `confirm_subscription_invites_warning.hbs`
to follow the naming convention.
Generally, we never want to recommend sudo for an operation that can
be done as a non-root user, and it's normal to configure Docker to be
usable by normal users.
We currently have created a copy of the
`clean_unused_caches.main` function in
`provision_inner.py` to clean the unused caches. But as
we have now converted the script into a python file we
can directly call that function.
This commit replaces that function (introduced in adc0ed4206) with
`clean_unused_caches.main`.
We split recent_topics module into recent_topics_(ui + data + util).
This allows us to reduce cyclical dependencies which were
created due to large list of imports in recent topics. Also, this
refactor on its own makes sense.
It's sufficiently tiny that the shared code benefits don't justify the
cost, given that we plan to move index.html to a different templating
system soon.
The current linter disallowed the pattern
where multiple refs were present without
an additional parameter in allOf, which should
be valid.
Fixed the condition to allow the change.
Steve Howell
Anders Kaseorg
Aman Agrawal
Mateusz Mandera
Palash
Sahil Batra
Alex Vandiver
Alya Abbott
Ganesh Pawar
YashRE42
Vishnu KS
Kevin Scott
rht
Kartik Soneji
Dinesh
Eeshan Garg
sahil839
Priyank Patel
Tim Abbott
Philipp Seßner
Priyansh Garg
Priyam Seth
Gaurav Pandey
manavdesai27
m-e-l-u-h-a-n
akshatdalton
Christalee
aryanshridhar
PIG208
Riken Shah
Suyash Vardhan Mathur