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 full Markdown support, syntax highlighting, collapsible spoilers and more. #2470

Open
wants to merge 9 commits into
base: clearnet
Choose a base branch
from

Conversation

ianmacd
Copy link

@ianmacd ianmacd commented Sep 5, 2022

Contributor checklist:

Description

This PR brings full Markdown support to Session, using the markdown-it module.

Additionally, a number of extensions to Markdown are implemented, namely typography (e.g. smart quotes), tables, sub- and superscripts, footnotes, insertions, marked highlights, abbreviations and collapsible spoilers (using a customer container).

Furthermore, full syntax highlighting of fenced blocks and in-line code is supported, courtesy of highlight.js.

This PR supersedes #2179 and fixes the three-year-old #405.

A picture is worth a thousand words, so here are several to illustrate how much is now possible:

Preformatted, syntax-highlighted text:
image

Syntax-highlighted program code, plus bold, italics and strikethrough:
image

Sub- and superscripts:
image

Marks and insertions:
image

Tables:
image

Footnotes:
image

Abbreviations:
image

Collapsible spoilers:
image
image

This new implementation of Markdown use the markdown-it module and is
considerably superior to my older homebrew implementation
(oxen-io#2179).

Advantages:

* More accurate, consistent rendering.

* HTML in messages is supported and properly rendered.

* URLs get linkified for free. The linkify-it module is still needed
  only when Markdown formatting is disabled in the Settings.

* Extensible. For example, collapsible spoiler sections and syntax
  highlighting for programming languages are easily added.

Outstanding issues:

* Linkified URLs invoke the URL handler without confirmation dialogue
  when clicked.

* This implementation still uses dangerouslySetInnerHTML, because I
  don't know how to use JSX.Element without marking up the marked-up
  text a second time. This means that the other message-modifying
  components — Emojify, Linkify and AddNewLines — are currently not
  chained.

  Note that Linkify and AddNewLines are effectively redundant when
  Markdown is activated anyway.

* Mark-up is displayed literally in quoted messages, and when messages
  are selected (e.g. for deletion).
  https://github.com/markdown-it/markdown-it-container

Example:

::: spoiler Click here to learn how it all ends
The hero dies!
:::
@zcyph
Copy link

zcyph commented Sep 5, 2022

Fantastic! Would love to see this merged. Thank you for all your contributions to this project, Ian.

@GrahamboJangles
Copy link

This would be awesome

@KeeJef
Copy link
Collaborator

KeeJef commented Sep 6, 2022

I think this is something we will need to do cross platform if we intend to include in Desktop, ill talk to the rest of the team about how this could work.

@ianmacd
Copy link
Author

ianmacd commented Sep 6, 2022

I think this is something we will need to do cross platform if we intend to include in Desktop, ill talk to the rest of the team about how this could work.

Definitely. That's a given, or some messages would look not just horrible in the other clients, but wouldn't even make sense in extreme cases.

The Markdown library/module used does the heavy lifting, and I imagine there are mature ones available for the mobile platforms. There will be subtle differences in implementation and supported feature set between the different libraries, but that's par for the course with a heterogenous code base like Session's.

@Bilb Bilb added the enhancement New feature or request label Sep 8, 2022
@KeeJef
Copy link
Collaborator

KeeJef commented Sep 8, 2022

I've spoken with the team about this, right now multi platform integration of full Markdown support is undesirable. Many members of the team were unconvinced that users would need something at rich as Markdown in a general purpose messaging app (do i really need to have footnotes or collapsibles in a message?).

However there was more general support for limited text formatting options, closer to something like #2179, basic formatting like Bold, underline, strikethrough, italics, and basic (non syntax highlighted) code blocks. This is something that we could put on the roadmap for all platforms in the future, although its not a priority right now.

@ianmacd
Copy link
Author

ianmacd commented Sep 8, 2022

I've spoken with the team about this, right now multi platform integration of full Markdown support is undesirable. Many members of the team were unconvinced that users would need something at rich as Markdown in a general purpose messaging app (do i really need to have footnotes or collapsibles in a message?).

No, you don't need footnotes. Probably no-one would use them. They're an extension to Markdown and it took me only 5 minutes to implement them, so it was more an illustration of what can be done than anything else.

As for expandable/collapsible message sections, however, yes, Session should absolutely have this functionality. Spoilers make it much more practical to run (open) groups devoted to jokes, films, books, sporting events and anything else that relies on the concealment of conclusory information.

We've had spoiler functionality in Usenet clients since at least the mid-nineties. There, embedding an ASCII form-feed character in a message is enough to make the client treat the text under it as a spoiler. And before that, we used ROT13.

Like footnotes, spoilers are not part of the Markdown spec, but their utility is undisputed. The fact that modern instant messaging clients don't support the feature is a testament to how impoverished the field now is. We don't have multi-level threading either, but not because the idea itself is bad. It's because software has been dumbed down over the years to appeal to the lowest common denominator.

Simplicity and accessibility are important goals, but they don't need to be pursued at the expense of power and flexibility.

Markdown is actually a trivial case in this regard, because adding support for it doesn't complicate an IM client at all. It sits there invisible to all who don't make use of it. There's no added complexity, no UI clutter, nothing.

Telegram has full Markdown support, as do Discord and Element. Element goes a step further in offering full syntax highlighting. This makes all three clients considerably superior to Session for users who need to include preformatted text snippets in their messages. And that category of users consists of much more than just programmers; it's also anyone who has ever needed to include the output of programs run at the command line, those who need to paste log files in the course of troubleshooting, and those who wish to present tabulated data in an easily digestible format.

However there was more general support for limited text formatting options, closer to something like #2179, basic formatting like Bold, underline, strikethrough, italics, and basic (non syntax highlighted) code blocks. This is something that we could put on the roadmap for all platforms in the future, although its not a priority right now.

This is a very (note the use of Markdown to emphasise that word) disappointing response. You yourself opened #405 3 years ago to request Markdown support. Obviously it hasn't been a priority, or we'd have it by now. ;-)

The addition of Markdown support would be a clear case of low-hanging fruit for the Session team. The gains would vastly outstrip the development effort.

As a non-TypeScript programmer, it took me a day to add this feature to the desktop client, and another day to refine it. All of the heavy lifting is done by the libraries. In return for that work, a richness of expression is gained that vastly expands what Session is capable of today.

If it was worth spending several months of work to add emoji reactions to Session so that users could express their thoughts and emotions with a turd or a peeled banana, how can it not be worth adding the ability to enhance the written word, which is the very bread and butter of messaging software?

@necro-nemesis
Copy link

I'm not yet convinced that inclusion of these features doesn't meet with objectives of a "general messaging app" as I am being informed that WhatsApp supports Markdown. Not that Session is attempting to be WhatsApp but WhatsApp is what I would consider to be the epitome of a general messaging app.

Are there concerns regarding the method in which the support was added or the workload required to add it cross platform?

@KeeJef
Copy link
Collaborator

KeeJef commented Sep 8, 2022

From what I can find Telegram and Whatsapp do not support full markdown, they support a limited number of text formatting options like bold, italics, strikethrough, monospacing, hyperlinks. But they do not support more exotic features like footnotes, collapsibles, syntax highlighting etc... As I said we are generally in favour of these more limited text formatting options.

@ianmacd
Copy link
Author

ianmacd commented Sep 8, 2022

From what I can find Telegram and Whatsapp do not support full markdown, they support a limited number of text formatting options like bold, italics, strikethrough, monospacing, hyperlinks. But they do not support more exotic features like footnotes, collapsibles, syntax highlighting etc... As I said we are generally in favour of these more limited text formatting options.

Footnotes, expandable/collapsible sections and syntax highlighting are not part of the Markdown spec. Indeed, syntax highlighting doesn't require any extra mark-up, since the whole premise of that feature is that the syntax itself is meaningful, and can therefore be parsed and tokenised.

So, providing "full Markdown support" doesn't require that these more exotic features be implemented, although when they are offered as extensions by the library you're chosen and there's no additional code complexity to consider, why wouldn't you include them, particularly syntax highlighting? Telegram and WhatsApp may not have that functionality, but why be only as good as the competition when you can instead be better?

Look at Discord and Element to see how well Markdown has been implemented there.

Incidentally, even strikethrough isn't officially part of Markdown, but I would advocate its inclusion, along with sub- and superscripts, as these all have common use cases in everyday discussion.

@ghost
Copy link

ghost commented Sep 8, 2022

I've spoken with the team about this, right now multi platform integration of full Markdown support is undesirable. Many members of the team were unconvinced that users would need something at rich as Markdown in a general purpose messaging app (do i really need to have footnotes or collapsibles in a message?).

However there was more general support for limited text formatting options, closer to something like #2179, basic formatting like Bold, underline, strikethrough, italics, and basic (non syntax highlighted) code blocks. This is something that we could put on the roadmap for all platforms in the future, although its not a priority right now.

I'm a group owner of a private tech community with 500 members on a proprietary platform.

I had been looking forward to migrating my community to Session for a while.

I'm sure syntax highlight is a nice feature to have for my group, however, I can only speak for my group, not for general users.

With that being said, I respect the dev team to make decisions on the actual priority of implementing any of these features.

@beantaco
Copy link

Is full Markdown support as proposed here really required, or is limited text formatting as proposed in #2179 good enough?

The pros of Markdown support are obvious, but before any Markdown support is added I would be curious to know what cons will be introduced. What dependencies would be introduced, and what are the performance and security implications?

@keybreak
Copy link

Superb! 👍

Only one thing to add here - it should work exactly the same for all clients (Android / iOS etc), coz now for example code blocks work on Desktop and doesn't work on Android 🙃

Full markdown specification is very welcome 🥳

@pottsandpans
Copy link
Collaborator

This is being tracked on Jira:
https://optf.atlassian.net/browse/SES-1692

@pottsandpans pottsandpans added the Jira This ticket is being tracked in Jira label Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Jira This ticket is being tracked in Jira
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants