Skip to content
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

compose: Quote and reply #116

Closed
chrisbobbe opened this issue May 26, 2023 · 4 comments · Fixed by #201
Closed

compose: Quote and reply #116

chrisbobbe opened this issue May 26, 2023 · 4 comments · Fixed by #201
Assignees
Labels
a-compose Compose box, autocomplete, attaching files/images a-msglist The message-list screen, except what's label:a-content
Milestone

Comments

@chrisbobbe
Copy link
Collaborator

No description provided.

@gnprice gnprice added a-msglist The message-list screen, except what's label:a-content a-compose Compose box, autocomplete, attaching files/images labels May 26, 2023
@gnprice gnprice added this to the Alpha milestone May 27, 2023
@gnprice gnprice removed the m-alpha label May 27, 2023
@chrisbobbe chrisbobbe self-assigned this May 27, 2023
@chrisbobbe
Copy link
Collaborator Author

It might be helpful to add a quote_and_reply.dart file somewhere. Generating the quote-and-reply text has some nontrivial subtasks:

  • Silent @ mention of the sender
  • Link to the message (different for PM and stream messages)
  • Fencing with ```, but with the right number of `'s depending on the message content
  • (Leading and trailing newlines, if they're considered part of the text.)

And a new file with that name could be a good home for all that code. Where might be a good place to put that file? Maybe not "model"…or "widgets"…etc.

@chrisbobbe
Copy link
Collaborator Author

Greg points out in the office just now, model would be a fine place for it.

@chrisbobbe
Copy link
Collaborator Author

#140 will be helpful as a way to get the message's raw Markdown content.

@chrisbobbe
Copy link
Collaborator Author

With #139 being merged, we can consult store.streams when writing a stream-message link. (Then when we update store.streams on stream events, tracked in #135, those links will naturally get written with up-to-date data.)

chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 12, 2023
We can use this to get raw Markdown content for quote-and-reply
(zulip#116) and for the "Share" option on a message. For those, we only
care about the raw Markdown content and so could just as well have
used the `raw_content` field on the get-single-message response, for
servers pre-120. But...

We can also use this for zulip#73, "Handle Zulip-internal links by
navigation", to follow /near/<id> links through topic/stream moves
(see implementation in zulip-mobile). For that, we'll need more than
just the message's raw Markdown.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
TODO tests.

For the corresponding logic used in the web app and zulip-mobile,
see web/shared/src/fenced_code.ts.

I believe the logic here differs from that in just this way: it
follows the CommonMark spec more closely by disqualifying
backtick-fence lines where the "info string" has a backtick, since
that's not allowed:

> If the info string comes after a backtick fence, it may not
> contain any backtick characters. (The reason for this restriction
> is that otherwise some inline code would be incorrectly
> interpreted as the beginning of a fenced code block.)

Regarding the new file lib/model/compose.dart, we do have existing
code that could reasonably move here, but it's pretty simple. It's
the code that gives the upload-file Markdown; see
registerUploadStart and registerUploadEnd in
[ContentTextEditingController].

Related: zulip#116
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
TODO tests.

For the corresponding logic used in the web app and zulip-mobile,
see web/shared/src/fenced_code.ts.

I believe the logic here differs from that in just this way: it
follows the CommonMark spec more closely by disqualifying
backtick-fence lines where the "info string" has a backtick, since
that's not allowed:

> If the info string comes after a backtick fence, it may not
> contain any backtick characters. (The reason for this restriction
> is that otherwise some inline code would be incorrectly
> interpreted as the beginning of a fenced code block.)

Regarding the new file lib/model/compose.dart, we do have existing
code that could reasonably move here, but it's pretty simple. It's
the code that gives the upload-file Markdown; see
registerUploadStart and registerUploadEnd in
[ContentTextEditingController].

Related: zulip#116
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
TODO tests.

For the corresponding logic used in the web app and zulip-mobile,
see web/shared/src/fenced_code.ts.

I believe the logic here differs from that in just this way: it
follows the CommonMark spec more closely by disqualifying
backtick-fence lines where the "info string" has a backtick, since
that's not allowed:

> If the info string comes after a backtick fence, it may not
> contain any backtick characters. (The reason for this restriction
> is that otherwise some inline code would be incorrectly
> interpreted as the beginning of a fenced code block.)

Regarding the new file lib/model/compose.dart, we do have existing
code that could reasonably move here, but it's pretty simple. It's
the code that gives the upload-file Markdown; see
registerUploadStart and registerUploadEnd in
[ContentTextEditingController].

Related: zulip#116
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
For the corresponding logic used in the web app and zulip-mobile,
see web/shared/src/fenced_code.ts.

I believe the logic here differs from that in just this way: it
follows the CommonMark spec more closely by disqualifying
backtick-fence lines where the "info string" has a backtick, since
that's not allowed:

> If the info string comes after a backtick fence, it may not
> contain any backtick characters. (The reason for this restriction
> is that otherwise some inline code would be incorrectly
> interpreted as the beginning of a fenced code block.)

Regarding the new file lib/model/compose.dart, we do have existing
code that could reasonably move here, but it's pretty simple. It's
the code that gives the upload-file Markdown; see
registerUploadStart and registerUploadEnd in
[ContentTextEditingController].

Related: zulip#116
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
We can use this to get raw Markdown content for quote-and-reply
(zulip#116) and for the "Share" option on a message. For those, we only
care about the raw Markdown content and so could just as well have
used the `raw_content` field on the get-single-message response, for
servers pre-120. But...

We can also use this for zulip#73, "Handle Zulip-internal links by
navigation", to follow /near/<id> links through topic/stream moves
(see implementation in zulip-mobile). For that, we'll need more than
just the message's raw Markdown.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
We can use this to get raw Markdown content for quote-and-reply
(zulip#116) and for the "Share" option on a message. For those, we only
care about the raw Markdown content and so could just as well have
used the `raw_content` field on the get-single-message response, for
servers pre-120. But...

We can also use this for zulip#73, "Handle Zulip-internal links by
navigation", to follow /near/<id> links through topic/stream moves
(see implementation in zulip-mobile). For that, we'll need more than
just the message's raw Markdown.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
We can use this to get raw Markdown content for quote-and-reply
(zulip#116) and for the "Share" option on a message. For those, we only
care about the raw Markdown content and so could just as well have
used the `raw_content` field on the get-single-message response, for
servers pre-120. But...

We can also use this for zulip#73, "Handle Zulip-internal links by
navigation", to follow /near/<id> links through topic/stream moves
(see implementation in zulip-mobile). For that, we'll need more than
just the message's raw Markdown.
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
We can use this to get raw Markdown content for quote-and-reply
(zulip#116) and for the "Share" option on a message. For those, we only
care about the raw Markdown content and so could just as well have
used the `raw_content` field on the get-single-message response, for
servers pre-120. But...

We can also use this for zulip#73, "Handle Zulip-internal links by
navigation", to follow /near/<id> links through topic/stream moves
(see implementation in zulip-mobile). For that, we'll need more than
just the message's raw Markdown.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 14, 2023
For the corresponding logic used in the web app and zulip-mobile,
see web/shared/src/fenced_code.ts.

I believe the logic here differs from that in just this way: it
follows the CommonMark spec more closely by disqualifying
backtick-fence lines where the "info string" has a backtick, since
that's not allowed:

> If the info string comes after a backtick fence, it may not
> contain any backtick characters. (The reason for this restriction
> is that otherwise some inline code would be incorrectly
> interpreted as the beginning of a fenced code block.)

Regarding the new file lib/model/compose.dart, we do have existing
code that could reasonably move here, but it's pretty simple. It's
the code that gives the upload-file Markdown; see
registerUploadStart and registerUploadEnd in
[ContentTextEditingController].

Related: zulip#116
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 14, 2023
For the corresponding logic used in the web app and zulip-mobile,
see web/shared/src/fenced_code.ts.

I believe the logic here differs from that in just this way: it
follows the CommonMark spec more closely by disqualifying
backtick-fence lines where the "info string" has a backtick, since
that's not allowed:

> If the info string comes after a backtick fence, it may not
> contain any backtick characters. (The reason for this restriction
> is that otherwise some inline code would be incorrectly
> interpreted as the beginning of a fenced code block.)

Regarding the new file lib/model/compose.dart, we do have existing
code that could reasonably move here, but it's pretty simple. It's
the code that gives the upload-file Markdown; see
registerUploadStart and registerUploadEnd in
[ContentTextEditingController].

Related: zulip#116
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 14, 2023
We'll use this soon, for quote-and-reply zulip#116.

With the recent commit 4e11ea7, PerAccountStoreTestExtension now
has `addStream`, which we use in the tests.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 15, 2023
We'll use this soon, for quote-and-reply zulip#116.

With the recent commit 4e11ea7, PerAccountStoreTestExtension now
has `addStream`, which we use in the tests.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 15, 2023
For the corresponding logic used in the web app and zulip-mobile,
see web/shared/src/fenced_code.ts.

I believe the logic here differs from that in just this way: it
follows the CommonMark spec more closely by disqualifying
backtick-fence lines where the "info string" has a backtick, since
that's not allowed:

> If the info string comes after a backtick fence, it may not
> contain any backtick characters. (The reason for this restriction
> is that otherwise some inline code would be incorrectly
> interpreted as the beginning of a fenced code block.)

Regarding the new file lib/model/compose.dart, we do have existing
code that could reasonably move here, but it's pretty simple. It's
the code that gives the upload-file Markdown; see
registerUploadStart and registerUploadEnd in
[ContentTextEditingController].

Related: zulip#116
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 15, 2023
We'll use this soon, for quote-and-reply zulip#116.

With the recent commit 4e11ea7, PerAccountStoreTestExtension now
has `addStream`, which we use in the tests.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 16, 2023
For the corresponding logic used in the web app and zulip-mobile,
see web/shared/src/fenced_code.ts.

I believe the logic here differs from that in just this way: it
follows the CommonMark spec more closely by disqualifying
backtick-fence lines where the "info string" has a backtick, since
that's not allowed:

> If the info string comes after a backtick fence, it may not
> contain any backtick characters. (The reason for this restriction
> is that otherwise some inline code would be incorrectly
> interpreted as the beginning of a fenced code block.)

Regarding the new file lib/model/compose.dart, we do have existing
code that could reasonably move here, but it's pretty simple. It's
the code that gives the upload-file Markdown; see
registerUploadStart and registerUploadEnd in
[ContentTextEditingController].

Related: zulip#116
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 16, 2023
We'll use this soon, for quote-and-reply zulip#116.

With the recent commit 4e11ea7, PerAccountStoreTestExtension now
has `addStream`, which we use in the tests.
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 16, 2023
For the corresponding logic used in the web app and zulip-mobile,
see web/shared/src/fenced_code.ts.

I believe the logic here differs from that in just this way: it
follows the CommonMark spec more closely by disqualifying
backtick-fence lines where the "info string" has a backtick, since
that's not allowed:

> If the info string comes after a backtick fence, it may not
> contain any backtick characters. (The reason for this restriction
> is that otherwise some inline code would be incorrectly
> interpreted as the beginning of a fenced code block.)

Regarding the new file lib/model/compose.dart, we do have existing
code that could reasonably move here, but it's pretty simple. It's
the code that gives the upload-file Markdown; see
registerUploadStart and registerUploadEnd in
[ContentTextEditingController].

Related: zulip#116
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 16, 2023
We'll use this soon, for quote-and-reply zulip#116.

With the recent commit 4e11ea7, PerAccountStoreTestExtension now
has `addStream`, which we use in the tests.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 16, 2023
And use it in the one place where we've been creating inline links.

This is a small amount of code, but drawing it out into a helper
gives a convenient place to write down its shortcomings.

We'll also use this for zulip#116 quote-and-reply, coming up.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 16, 2023
We're about to add another button, for quote-and-reply zulip#116.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 16, 2023
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 17, 2023
And use it in the one place where we've been creating inline links.

This is a small amount of code, but drawing it out into a helper
gives a convenient place to write down its shortcomings.

We'll also use this for zulip#116 quote-and-reply, coming up.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 17, 2023
We're about to add another button, for quote-and-reply zulip#116.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 17, 2023
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 23, 2023
And use it in the one place where we've been creating inline links.

This is a small amount of code, but drawing it out into a helper
gives a convenient place to write down its shortcomings.

We'll also use this for zulip#116 quote-and-reply, coming up.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 23, 2023
We're about to add another button, for quote-and-reply zulip#116.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 23, 2023
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 27, 2023
We're about to add another button, for quote-and-reply zulip#116.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 27, 2023
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 27, 2023
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 28, 2023
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 28, 2023
And use it in the one place where we've been creating inline links.

This is a small amount of code, but drawing it out into a helper
gives a convenient place to write down its shortcomings.

We'll also use this for zulip#116 quote-and-reply, coming up.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 28, 2023
We're about to add another button, for quote-and-reply zulip#116.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 28, 2023
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this issue Jul 1, 2023
And use it in the one place where we've been creating inline links.

This is a small amount of code, but drawing it out into a helper
gives a convenient place to write down its shortcomings.

We'll also use this for zulip#116 quote-and-reply, coming up.
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this issue Jul 1, 2023
We're about to add another button, for quote-and-reply zulip#116.
@gnprice gnprice closed this as completed in 406b7d0 Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-compose Compose box, autocomplete, attaching files/images a-msglist The message-list screen, except what's label:a-content
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants
@gnprice @chrisbobbe and others