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

Add Labelled hashtag #69

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add Labelled hashtag #69

wants to merge 7 commits into from

Conversation

farooqkz
Copy link
Collaborator

@farooqkz farooqkz commented May 9, 2024

No description provided.

src/parser/mod.rs Outdated Show resolved Hide resolved
@Simon-Laux
Copy link
Member

don't forget to add the specification for the new element to spec.md. It will be desktop- and markdown-set element.

@farooqkz
Copy link
Collaborator Author

don't forget to add the specification for the new element to spec.md. It will be desktop- and markdown-set element.

Do we need labelled hashtags in desktop and markdown sets, too?

@farooqkz farooqkz marked this pull request as ready for review May 29, 2024 08:34
@@ -23,6 +23,7 @@
- [Delimited Email addresses: `<hello@delta.chat>`](#delimited-email-addresses)
- [Delimited Links: `<http://example.org>`](#delimited-links)
- [Labeled Links: `[Name](url)`](#labled-links)
- [Labeled hashtags: `[Tag][#tag]`](#labeled-tags)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we also want to have this in the desktop set?

### Labelled hashtags

The idea is to have hashtags but labelled with an alternative text. This feature is very unique and less seen in other IMs.

Copy link
Member

@Simon-Laux Simon-Laux May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please also make an example and spec to define how it looks like:

[Label](#tag)

and say what it does: "clicking on it opens the search with that hashtag like for the normal hashtag but looks like a link", or sth along those lines

src/main.rs Outdated
Copy link
Member

@Simon-Laux Simon-Laux May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file change seems unrelated, I guess we should turn this into a full cli-app at some point where you can select the mode via argument. (maybe with the clap library)

Comment on lines +158 to +159
if let Ok((i, elm)) = labelled_tag(input) {
Ok((i, elm))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fabled tag should only be in the markdown set and maybe also in the desktop set, but not in the text set.
rule of thumb: Things in the text set are kept at the original, just linkified, but text is still the same

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You sure? hashtag is already in text elements.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, labeled link and labeled hashtag both modify the text, while text elements only make the text clickable

Copy link
Member

@Simon-Laux Simon-Laux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should only be part of markdown and (maybe, if you think it makes sense, I think it kinda does make sense because desktop set also has labeled links already) desktop set, but not the text set

take_while1(|c| !matches!(c, '[' | ']')),
char(']'),
)(input)?;
let elements: Vec<Element> = parse_only_text(label);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we merge #74 first, then we can allow markdown formatting for the thing in markdown mode and in desktop mode this should just allow text. So we don't repeat #59 (there should no be any clickable elements in the label, otherwise it's confusing/not-good-defined what happens)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants