Handle internal links #708

preetmishra · 2020-07-04T19:18:26Z

The PR adds support for handling internal links - stream, topic and near narrow.

Commits

The commits introduce MessageLinkButton in MsgInfoView and then incrementally add support for different narrows.

I would greatly appreciate any feedback on the changes, the commit chronology and the structure in general.

Partially fixes #352 and #764.

zulipterminal/ui_tools/buttons.py

preetmishra · 2020-07-22T11:57:51Z

I was meaning to change the esc focus for successful internal link narrows but I see the current behaviour aligns with esc key behaviour that we already have elsewhere (e.g. web app and zulip-term itself), that is, esc transitions to 'All messages' narrow no matter in which narrow you are in at that moment. Thoughts?

zulipterminal/ui_tools/views.py

sumanthvrao

@preetmishra Reviewed the first three commits and left one comment. Besides that the first 3 commits look good if we want to merge them separately.

zulipterminal/core.py

zulipterminal/ui_tools/views.py

preetmishra · 2020-07-23T05:40:17Z

@sumanthvrao Thanks for the review. As discussed in the PM, the first three commits are merged! 🎉

I have pushed a rebased version for further review.

zulipbot · 2020-07-31T14:48:28Z

Hello @preetmishra, it seems like you have referenced #352 in your pull request description, but you have not referenced them in your commit message description(s). When you reference an issue in a commit message, it automatically closes the corresponding issue when the commit is merged.

Please run git commit --amend in your command line client to amend your commit message description with Fixes #352..

An example of a correctly-formatted commit:

commit fabd5e450374c8dde65ec35f02140383940fe146
Author: zulipbot
Date:   Sat Mar 18 13:42:40 2017 -0700

    pull requests: Check PR commits reference when issue is referenced.

    Fixes #51.

Thank you for your contributions to Zulip!

preetmishra · 2020-07-31T14:48:46Z

Updated to resolve conflicts.

preetmishra · 2020-08-05T17:06:14Z

Updated with tests.

preetmishra · 2020-08-15T19:24:19Z

Updated with tests and rebase onto upstream/master.

neiljp

@preetmishra This looks great! It's a little verbose due to type handling and few other aspects, which is the last thing I'd like to see improve (pending reviewing of tests).

I think we can simplify the type notes sprinkled throughout using some TypedDict use, which will make the code a lot more concise and readable. There are a few other ideas inline on how we can compact the code.

Since I've given a few notes on the types and data being passed around, I've held off on the reviewing the tests for now.

zulipterminal/ui_tools/buttons.py

preetmishra · 2020-08-20T13:44:50Z

@neiljp Thanks for the suggestions. They make the code clearer and concise.

In the latest update, I have incorporated the suggestions that you left in-line and amended the tests accordingly.

preetmishra · 2020-08-21T10:43:11Z

Updated to resolve conflicts.

This introduces handle_narrow_link(), along with a couple of helper methods, to handle narrow links and support stream narrow in MessageLinkButton. Helper methods: * _parse_narrow_link(). * _decode_stream_data(). * _validate_narrow_link(). * _validate_and_patch_stream_data(). * _switch_narrow_to(). Additionally, this adds a helper method, hash_util_decode(), in helper.py to decode server URL excerpts. Tests added.

This extends the existing helper methods, _parse_narrow_link(), _validate_narrow_link() and _switch_narrow_to(), to add support for topic narrow. Tests amended.

This extends the existing helper methods, _parse_narrow_link(), _validate_narrow_link() and _switch_narrow_to(), and adds _decode_message_id() to add support for near stream and near topic narrow. The near operator is used to set focus to a particular message (using the anchor) while switching narrow. Tests amended.

preetmishra · 2020-08-21T18:26:51Z

Updated to resolve conflicts.

neiljp · 2020-08-29T04:01:11Z

@preetmishra Thanks for working on this - this is another feature that will make ZT easier to use for jumping between streams/topics 👍 This took a while to review due to the size, and there are a few refactors we can do to improve this, but I'm going to merge this now so we can get it out for wider view 🎉

zulipbot added the size: XL [Automatic label added by zulipbot] label Jul 4, 2020

preetmishra commented Jul 4, 2020

View reviewed changes

zulipterminal/ui_tools/buttons.py Outdated Show resolved Hide resolved

preetmishra force-pushed the handle-internal-links branch 3 times, most recently from 1865ad0 to 90a7016 Compare July 21, 2020 18:18

preetmishra force-pushed the handle-internal-links branch from 90a7016 to a2d980f Compare July 22, 2020 12:28

preetmishra marked this pull request as ready for review July 22, 2020 12:39

preetmishra added the PR needs review PR requires feedback to proceed label Jul 22, 2020

preetmishra mentioned this pull request Jul 22, 2020

Links that break into a new line don't get recognized completely #622

Open

preetmishra commented Jul 22, 2020

View reviewed changes

zulipterminal/ui_tools/views.py Outdated Show resolved Hide resolved

neiljp added this to the Release after upcoming milestone Jul 22, 2020

preetmishra force-pushed the handle-internal-links branch 2 times, most recently from 7c4d7df to 3a0dade Compare July 22, 2020 19:28

sumanthvrao reviewed Jul 23, 2020

View reviewed changes

zulipterminal/core.py Outdated Show resolved Hide resolved

zulipterminal/ui_tools/views.py Outdated Show resolved Hide resolved

preetmishra force-pushed the handle-internal-links branch from 3a0dade to 1223ad1 Compare July 23, 2020 05:36

zulipbot added size: L [Automatic label added by zulipbot] and removed size: XL [Automatic label added by zulipbot] labels Jul 23, 2020

This was referenced Jul 23, 2020

TRACKING: Message rendering bugs/enhancements #452

Open

[WIP] Handle media links #740

Closed

preetmishra changed the title ~~[WIP] Handle internal links~~ Handle internal links Jul 29, 2020

preetmishra force-pushed the handle-internal-links branch from 1223ad1 to f03ef65 Compare July 31, 2020 14:48

preetmishra force-pushed the handle-internal-links branch from f03ef65 to 82c6867 Compare August 5, 2020 17:01

zulipbot added size: XL [Automatic label added by zulipbot] and removed size: L [Automatic label added by zulipbot] labels Aug 5, 2020

preetmishra mentioned this pull request Aug 6, 2020

TRACKING: Handle message links #764

Open

4 tasks

zulipbot added the has conflicts label Aug 14, 2020

preetmishra removed PR blocks other PR feedback wanted labels Aug 15, 2020

preetmishra force-pushed the handle-internal-links branch 2 times, most recently from ec28c38 to 084da38 Compare August 15, 2020 19:15

preetmishra changed the title ~~[WIP] Handle internal links~~ Handle internal links Aug 15, 2020

preetmishra added PR needs review PR requires feedback to proceed and removed has conflicts labels Aug 15, 2020

neiljp reviewed Aug 17, 2020

View reviewed changes

neiljp removed the PR needs review PR requires feedback to proceed label Aug 17, 2020

preetmishra changed the title ~~Handle internal links~~ [WIP] Handle internal links Aug 17, 2020

preetmishra force-pushed the handle-internal-links branch 2 times, most recently from 9e1946f to 18548a7 Compare August 20, 2020 13:33

preetmishra commented Aug 20, 2020

View reviewed changes

zulipterminal/ui_tools/buttons.py Outdated Show resolved Hide resolved

preetmishra changed the title ~~[WIP] Handle internal links~~ Handle internal links Aug 20, 2020

preetmishra added the PR needs review PR requires feedback to proceed label Aug 20, 2020

preetmishra force-pushed the handle-internal-links branch from 18548a7 to caefdf6 Compare August 21, 2020 10:21

preetmishra added 3 commits August 21, 2020 23:50

buttons: Add support for topic narrow in MessageLinkButton.

73cc21e

This extends the existing helper methods, _parse_narrow_link(), _validate_narrow_link() and _switch_narrow_to(), to add support for topic narrow. Tests amended.

preetmishra force-pushed the handle-internal-links branch from caefdf6 to 330cef9 Compare August 21, 2020 18:23

neiljp merged commit 6ccac96 into zulip:master Aug 29, 2020

preetmishra deleted the handle-internal-links branch August 29, 2020 05:19

Ezio-Sarthak mentioned this pull request Jan 21, 2021

Handle internal PM links #883

Open

zee-bit mentioned this pull request Mar 24, 2021

Narrowing with a contextual message id should select that message #954

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle internal links #708

Handle internal links #708

preetmishra commented Jul 4, 2020 •

edited

Loading

preetmishra commented Jul 22, 2020

sumanthvrao left a comment

preetmishra commented Jul 23, 2020

zulipbot commented Jul 31, 2020

preetmishra commented Jul 31, 2020

preetmishra commented Aug 5, 2020

preetmishra commented Aug 15, 2020

neiljp left a comment

preetmishra commented Aug 20, 2020

preetmishra commented Aug 21, 2020

preetmishra commented Aug 21, 2020

neiljp commented Aug 29, 2020

Handle internal links #708

Handle internal links #708

Conversation

preetmishra commented Jul 4, 2020 • edited Loading

Commits

preetmishra commented Jul 22, 2020

sumanthvrao left a comment

Choose a reason for hiding this comment

preetmishra commented Jul 23, 2020

zulipbot commented Jul 31, 2020

preetmishra commented Jul 31, 2020

preetmishra commented Aug 5, 2020

preetmishra commented Aug 15, 2020

neiljp left a comment

Choose a reason for hiding this comment

preetmishra commented Aug 20, 2020

preetmishra commented Aug 21, 2020

preetmishra commented Aug 21, 2020

neiljp commented Aug 29, 2020

preetmishra commented Jul 4, 2020 •

edited

Loading