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

issues with links in brackets #12

Closed
r10s opened this issue Dec 20, 2021 · 5 comments · Fixed by #17
Closed

issues with links in brackets #12

r10s opened this issue Dec 20, 2021 · 5 comments · Fixed by #17
Labels
bug Something isn't working

Comments

@r10s
Copy link
Member

r10s commented Dec 20, 2021

typing a link in brackets is not working, eg. sth. as A great chat app (see https://delta.chat/en/)

unfortunately, this is contradictory to deltachat/deltachat-desktop#2238

example ...

A great chat app (see https://delta.chat/en/) vs. https://en.wikipedia.org/wiki/Bracket_(disambiguation)

... markdown on github does it right, also ios (did not test android yet). technically, the second link is wrong afaik, however, in practise we cannot insist on that, these links just exist in masses :)

another bug i found during testing ...

a great chat app (see https://delta.chat) is not linkified at all.

@Jikstra
Copy link

Jikstra commented Dec 20, 2021

So basically not allowing a ')' after '/' if the ')' is the last character?

@r10s
Copy link
Member Author

r10s commented Dec 20, 2021

So basically not allowing a ')' after '/' if the ')' is the last character?

not sure, these things are tricky. i would not rely on /. i would have a look what other implementations are doing here :)

trying to get such things correct "from scratch" is probably a huge pain, issues will pop up over and over.

@Simon-Laux
Copy link
Member

maybe also report on https://github.com/deltachat/message-parser/issues with some test cases (text, expected result, actual result)

looks like this could be solved with context awareness, so only consume as many brackets as were opened.
Or an other option would be a pseudo parsing element that parses brackets: delimited('(', many(anyElement), ')'), so that the link cannot eat brackets that are closing other ones. but also delimited is not enough for nested brackets, so we would need to implement a bracket parser ourselves. not sure what the better solution is, anyways the most important thing is that we add these things as unit tests so what ever the code might be doing as long as we have enough tests it has to work.

@r10s
Copy link
Member Author

r10s commented Jan 10, 2022

So basically not allowing a ')' after '/' if the ')' is the last character?

just got a real-life example in the dev chat where the bracket is not directly after a slash ...

for desktop we use "mergable" for this (https://github.com/deltachat/deltachat-desktop/blob/master/.github/mergeable.yml), but it is ...

... renders wrongly in delta chat 1.26.0

@Simon-Laux
Copy link
Member

a small update:

  • android also has this wrong behaviour
  • WhatsApp does it right (they seem to count/match the brackets. as I suggested above)

@Simon-Laux Simon-Laux transferred this issue from deltachat/deltachat-desktop Jan 17, 2022
@Simon-Laux Simon-Laux added the bug Something isn't working label Jan 17, 2022
Simon-Laux added a commit that referenced this issue Mar 4, 2022
Simon-Laux added a commit that referenced this issue Mar 4, 2022
Simon-Laux added a commit that referenced this issue Mar 4, 2022
which I forgot to fix in the last commit:
starting a bracket at the end of a link:
`https://delta.chat/page(this is the link to our site)`
(when it is not closed in the link its not taken)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants