-
Notifications
You must be signed in to change notification settings - Fork 191
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
base: unstable
Are you sure you want to change the base?
Conversation
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! :::
Fantastic! Would love to see this merged. Thank you for all your contributions to this project, Ian. |
This would be awesome |
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. |
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. |
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.
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? |
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? |
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. |
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. |
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? |
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 🥳 |
This is being tracked on Jira: |
Contributor checklist:
clearnet
branchyarn ready
run passes successfully (more about tests here)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:
Syntax-highlighted program code, plus bold, italics and
strikethrough:Sub- and superscripts:
Marks and insertions:
Tables:
Footnotes:
Abbreviations:
Collapsible spoilers: