-
-
Notifications
You must be signed in to change notification settings - Fork 261
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
boxes/helper: Updated to new quoting style. #854
Conversation
6555660
to
ceb4711
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zee-bit Initial testing showed this to render fine, though possibly some topics with /
in them don't work?
You have the updated quote style ok, but as we discussed in the stream the challenge is how to handle url generation. I'd suggest decoupling the message-to-url part (helper.py is fine for now) as you have done here (from the url-generation part, in another file). The server has url_encoding.py
, but it uses other types connected to the server implementation like Realm
and it uses f-strings (though we plan to to move to that soon with python3.6). My suggestion would be to use the server file as a basis for a module in our repo which handles chat server urls like this, and we can incorporate your and @preetmishra 's work if it builds upon that in a useful way.
Other than interoperability and tracking changes in the server, the reason I'd like this in a separate file is since we both construct and deconstruct server URLs from narrows and it'd be useful to have it together, and also is related to my hope that we could later integrate this into the python API package.
We can discuss more in the stream if you like.
@zulipbot add "PR needs review" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zee-bit This is promising, but as per another PR we can tidy this up a little more in terms of commit structure :)
It'll be great to have the bugfix, but as with the other code I'd like to see text of how it was translated across from the server (if it was), and also some tests should help with indicating and checking behavior 👍
This commit is a bug-fix that corrects generation of fences for quoted messages. The logic for generation of quote fence is borrowed from the webapp. Fences are generated after searching for maximum length occurence of ` through regex, and wrapping the quote block with the generated fence. This allows for proper quoting of messages containing a code-block or another quote inside them. This is the first part of commit for this Pull-request. Tests amended.
This is the second commit to this PR, which adds the same quoting style as used in the webapp. The URL generation part of the code now has its own module named server_url.py along with the tests for this module added as test_server_url.py. This will allow to track changes in the server and make constructing and deconstructing server URL's easier. This module can also be later integrated into the API package. Tests amended. Fixes zulip#514.
Adds the new module server_url.py along with its description in the developer-documentation.
Heads up @zee-bit, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
This PR aims to bring ZT one step more closer to its web counterpart. The quoting style is changed to include the name of the user whose message was quoted(and his
user_id
) along with a link that would narrow to the quoted message. This is the new markdown/bugdown syntax of a quoted message:For interoperability, the link to the quoted message is the same as that of the web app, so narrowing to the quote in ZT is possible.
Also, the URL generation and encoding part of the code has been shifted to a new module named
server_url.py
to separate it from the rest of the code insidehelper.py
and make it easier for constructing and deconstructing server URL in the future.This PR should fix #514 and #151(partially).