[WIP] Handle typing events from multiple users in a PM narrow #1291

The strict section now matches entries and ordering from mypy --strict. Other general checking options are extracted into their own sections according to the order in the mypy --help page, except for those already in the strict section. Where the main command-line flag loosens the type-checking, these options are treated as mypy defaults, and commented in each section. This commit also starts enforcing the following, which require no changes to code: - strict_concatenate [strict] - truthy-iterable [error-code] - ignore-without-code [error-code] - unused-awaitable [error-code]

This is part of the strict block of checks. This change passes mypy in a local full development environment, but to pass in GitHub Actions the test dependencies must now be added to the type-checking dependencies.

Confirmed after identification from mypy with warn_unreachable option.

These changes ensure that mypy will not flag branches of code as unreachable. One type could be inferred instead, but is amended and remains to help with readability, in _fetch_user_data.

This enforces this check after adjustments in previous commits. This is a warning not included in mypy strict mode, so is added to a new section.

These hard-coded options are now better stored in the pyproject.toml configuration, while command-line options have not changed in some time and are no longer necessary.

lister.py was previously explicitly excluded from type-checking, but if we remove the hard-coded `follow-imports=silent` in tools/run-mypy, it is evident that it was previously being checked due to being imported where it was used - but error messages were not reported. With minimal typing and a minor change to lister.py, the file can be made to pass type-checking, enabling an overall move to the better default `follow-imports=normal` setting in pyproject.toml. Moreover, this also allows lister.py to be removed from the list of files excluded from type-checking we maintain in pyproject.toml and run-mypy.

This was surfaced by the use of the type stubs for beautifulsoup4. Notes on the changes: - tag_attr.get("attribute", []) can return a str or List[str] at runtime, depending upon whether the tag can be multivalued. Using .get_attribute_list() instead makes this explicit, in the absence of type stubs for .get() overloaded on the name of the parameter. - Similarly, "title" is a single-entry field, so it is more accurate to set the default to the empty string. - Lastly, .find() only applies to a Tag, and can result in a Tag, but also potentially None (already checked) or a NavigableString. Based on the structure of the document the latter is unlikely, but this is now ensured by converting the not-None check to whether the result is explicitly a Tag.

Rather than mypy simply ignoring any missing type information on imports, this change sets this to be an error by default. Separate sections for imports used in the main application and only during development are added, to exclude certain libraries (import patterns) from resulting in errors, where the type information is not available. This makes the lack of typing more explicit and limited. In addition, an import in tools/convert-unicode-emoji-data is marked with a type-ignore comment, since the imported file is only temporary.

Specific lines involving ModListWalker are ignored, since this class has an implementation of __len__ in the base class, but that base class is identified as Any due to limited type information at this time.

An always-true condition in _handle_user_settings_event is moved from a conditional into an assert to satisfy this check.

Other than shortening the chain of imports, this makes it clearer where Message is originally defined, and that it is considered part of the API (api_types.py), not an internal structure (helper.py).

In general this behavior is now handled in pyproject.toml instead, but in certain cases this is not possible. This approach enables options to be applied differently when running mypy on repos/projects (folders) which interact, eg. import from each other, and changing one folder of files would otherwise be necessary *only* to satisfy mypy when running on the other.

This is enabled primarily in pyproject.toml. The flags used when applying mypy to tests/ (in run-mypy) are specifically adjusted to avoid this option. This cannot be achieved by applying an override to "tests.*" in pyproject.toml, since the error is flagged as being in imported non-test files. The source could be adjusted to take this into account, but it appears cleaner to import from the file being tested. Two names imported from the zulip library are explicitly exported from api_types.py to enable their use elsewhere in the source, using __all__. The alternative is to use the `from X import Y as Y` technique, but this triggers ruff error PLC0414 and is verbose.

Some fields were previously only defined in Model.get_all_users, but it is cleaner to initialize them to sane values prior to that method call. Others were defined above that call, but not directly, making it less clear that they were directly updated by it.

This method does currently return some data, but also updates other data as a side-effect, and is now only used internally to synchronize users data from 'initial data' from API register() calls, or subsequent updates to that data from events. The revised name of _update_users_data_from_initial_data reflects these factors. Tests updated.

StreamAccessType and UserStatus are internal types used to key into ui_mappings dicts. Since they are not part of the UI or Zulip API directly, other files seeking to import them would be better served importing from helper.py instead of the UI-centric mappings file.

Fixture and tests updated.

_update_users_data_from_initial_data also updates other model attributes, so it's clearer to not treat it differently. Tests updated.

This improves the message UI by adding stream access type markers as prefixes to stream names, when rendering headers of stream messages in the messages view. This gives users an easier way to determine what type of stream the message belongs to. Tests and fixtures are updated accordingly.

The previous commit added a prefix marker for headers of stream messages; this commit does the same for headers of direct messages. No symbol was previously defined for direct messages, so one is added, and applied in a similar way as with stream messages. Tests updated.

This commit adds a stream_topic_from_message_id method which returns the topic of the currently focused message. This is done in preparation of the change in algorithm of the get_next_unread_topic function. Tests added.

This commit changes the behavior of the get_next_unread_topic method to use the current message state (ie. the current topic) in calculating the next unread topic to be cycled to. If there is no current message available, ie. an empty narrow with no focus, the logic attempts to use the current narrow to determine the best course of action. This replaces the previous approach of using a _last_unread_topic stored in the model. Tests updated.

This commit renames the "get_next_unread_topic" function to "next_unread_topic_from_message_id". This makes the function name clearer by indicating that the function argument is the context from which we get the next unread topic. Tests updated to rename the function.

The next_unread_topic_from_message_id function returns the next topic to be narrowed to. If the topic remains same, there is no need to call narrow to the same topic again. This commit fixes this, without any change in user-facing behavior. Test case updated.

This commit aims to introduce in-stream wrap-around behavior to the next_unread_topic_from_message_id function if there are unread messages still present in the current stream. Test case added.

Hotkeys document updated.

This commit shifts test IDs for test__parse_narrow_link() to be inline with the test cases, and extracts SERVER_URL into the test function body.

This commit provides support for narrow links in message content containing 'subject' instead of 'topic', which may be present in messages before server version 2.1.0. Test cases added. Fixes zulip#1422.

Minor change to reduce confusion between values of different ids in test cases and ensure they are kept distinct for testing purposes.

This picks out O_WRONLY as a misspelling, but this version of typos allows configuration in pyproject.toml, so exclude it there.

This is achieved through a new method associate_stream_with_topic in the View, which saves to a new internal dict. This will support later restoring the topic position in the UI. Since the topic list may be reordered between saving and restoring the state, the current index in the topic list is insufficient; the name of the topic is used intead.

Information regarding the previous position in the topic list for the matching stream is retrieved from the View and used to set the initial focus. A new accessor in the View, saved_topic_in_stream_id, returns the most recent (saved) topic name. In the absence of a previous value this returns None. A new internal TopicsView helper method returns the focus (index) which corresponds to the saved topic name for the related stream. If there is no saved state or no matching topic name, it returns the top index (0), which was the previous behavior. This is isolated as a method to facilitate testing. Combined, this restores any previous topic-name state by assigning to the focus position. Test added for the internal helper method _focus_position_for_topic_name.

This commit introduces the sort_unread_topics function to the next_unread_topic_from_message_id method to sort unread_topics instead of sorting it in the model. This replaces the use of bisect since the sorting behavior changes due to the change in the sort_unread_topics function. The caveat for this replacement is that the sort is performed again. This commit serves as a preparatory commit for the change in sorting behavior in the next commit. Test added.

This commit changes the sort_unread_topics method in helper.py to use the left stream panel ordering to sort the unread data instead of implicitly using the stream id as key. Tests updated & extended.

The 'Mentioned messages' and 'Starred messages' symbols are set to ASCII characters, similar to those in the Zulip web app. The selected 'All messages' symbol is the closest match found after some testing, and appears to be available fairly widely. An appropriate descriptive comment is added, as with other non-ASCII symbols, for later reference.

This commit adds prefix symbols to the main buttons in the top left corner of the UI. This aims to make the main buttons stand out more and approximate the designs used in the web app. This change also leads to a cleaner design, since these top buttons are now indented and aligned similarly to the panels beneath them, eg. the list of streams. The symbol used as a prefix in headers of direct messages is reused for the 'Direct messages' button, with other buttons using symbols added in the previous commit. The left part of the UI is increased in width to accommodate the new additions. Note that the "title" style is used to make the icons bolder, though this should be decoupled in future.

Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold strong' style via pygments as a result, which urwid fails to parse and blocks the application from loading. Longer-term we should improve the pygments to urwid translation logic to allow these styles to work and an upgrade to later pygments versions, but for now this allows these themes to continue working as before. Fixes zulip#1431.

The active_converstation_information is a Dict where id of the users is the key and the full_name of the user is the value. Previously the indexing was done by name which is changed to id now because of the possibility of non-unique names present in the active_conversation_info. In core.py active_conversation_info gets the full_names of the emails present in self.active_conversation_info's values.

Add a single user and remove a single user on typing event. Previously removing means emptying the dict as dict would contain a single user.But now removing means discarding a specfic id. In core.py diffrent cases of footer are made depending on no of active user. In test_core, the test now has it's own user_dict which is used to extract name for footer notifcation out of emails present in active_conversation_info. In test_model,added a new parameter of current active_conversation_info.

Add diffrernt cases of footer notification.Earlier just one test was presnt as not much scenarios were possible.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Handle typing events from multiple users in a PM narrow #1291

[WIP] Handle typing events from multiple users in a PM narrow #1291

Commits on Oct 27, 2023

[WIP] Handle typing events from multiple users in a PM narrow #1291

Are you sure you want to change the base?

[WIP] Handle typing events from multiple users in a PM narrow #1291

Commits on Oct 27, 2023