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

Markdown support #4076

Merged
merged 1 commit into from Apr 22, 2020
Merged

Markdown support #4076

merged 1 commit into from Apr 22, 2020

Conversation

Krzmbrzl
Copy link
Member

Supported are sections, inline code and code-blocks, bold, italic and
strikethrough text, links and quotes.

It is also possible to use the backslash character to escape other
characters (to prevent them from triggering Markdown interpretation).

The changes made in this commit do not apply to direct messages though
(which are assembled via a RichTextEditor and I haven't found a clever
way to hack into that one yet).

This implements #2945 and fixes #4075


An example:

# Markdown
## Code
We can have `inline` code and
```code
  blocks```

## Emphasis
**bold**  and *italic*  text is also supported. ~~Scratch that~~

## Links
We can have [links](google.de) and www.google.de

## Quotes
> I am a quote
Although these are not pretty, they do work and are supported.

## Escaping
We can escaoe markdown using the \\ character: \*not italic\* and \*\*not bold\*\*

yields
Mumble_Markdown


Changelog

| Added: Markdown support for TextMessages

@Krzmbrzl Krzmbrzl linked an issue Apr 18, 2020 that may be closed by this pull request
@Krzmbrzl Krzmbrzl added client feature-request This issue or PR deals with a new feature labels Apr 18, 2020
@Krzmbrzl Krzmbrzl added this to the 1.4.0 milestone Apr 18, 2020
@Johni0702
Copy link
Contributor

I believe it's generally expected that the zeroth line of code blocks (i.e. the remainder of the line after ```) be ignored (or used to specify the language). See e.g. GitHub's Markdown spec: https://github.github.com/gfm/#fenced-code-blocks

E.g.

```
test
```

should render as as single line:

test

not as two lines:


test

and

```java
test
```

should render just like the first example with optional syntax highlighting (I'd consider that part out of scope of this PR though), not as

java
test

@Kissaki
Copy link
Member

Kissaki commented Apr 18, 2020

How does this fix #4075? Markdown does not typically render " quoted text as italic.

Copy link
Member

@Kissaki Kissaki left a comment

Choose a reason for hiding this comment

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

I would definitely put the Markdown parsing logic into it’s own class.

TextMessage is a ui component, and Markdown conversion is quite a distinct functionality.

@Krzmbrzl
Copy link
Member Author

@Kissaki the issue states that although the functionality for quotes is broken, it is good the way it is.

This PR has removed the respective code area altogether and provided proper support for quotations. That's why I think this closes the issue.

@bkacjios
Copy link

This is great! I was going to try my hand at doing this today, but I guess I don't have to now.

I only reported #4075 as an issue because I really didn't like the functionality, especially since it wasn't documented anywhere and there was no way to escape it. This implementation is more inline with what I'd expect, so I'm all for it.

@Krzmbrzl
Copy link
Member Author

@Kissaki I moved all markdown-related code to its own file (no need for a class though)

@Johni0702 good catch! I adapted the code accordingly

@vimpostor
Copy link
Contributor

BTW Qt already has builtin support for Markdown: https://doc-snapshots.qt.io/qt5-5.14/qtextedit.html

@Krzmbrzl
Copy link
Member Author

BTW Qt already has builtin support for Markdown: https://doc-snapshots.qt.io/qt5-5.14/qtextedit.html

Yes but only from 5.14 onwards. Until this is the version that is supported by the oldest still supported Ubuntu release, it'll probably take quite some time. And before that we can't make use of it.

@TredwellGit

This comment has been minimized.

@TredwellGit

This comment has been minimized.

@Krzmbrzl
Copy link
Member Author

As a reference: Qt's implementation of Markdown stuff seems to live in https://github.com/qt/qtbase/tree/5.14.0/src/gui/text

…ages

Supported are sections, inline code and code-blocks, bold, italic and
strikethrough text, links and quotes.

It is also possible to use the backslash character to escape other
characters (to prevent them from triggering Markdown interpretation).

The changes made in this commit do not apply to direct messages though
(which are assembled via a RichTextEditor and I haven't found a clever
way to hack into that one yet).

This implements mumble-voip#2945 and fixes mumble-voip#4075
@Krzmbrzl Krzmbrzl merged commit 97b3456 into mumble-voip:master Apr 22, 2020
@fedetft
Copy link
Contributor

fedetft commented May 15, 2020

Hi, I tested this new feature, and it's great!

However, I think I found a bug, the < and > characters in markdown code blocks get replaced by &lt; and &gt;

@Kissaki
Copy link
Member

Kissaki commented May 16, 2020

@fedetft thank you for reporting back. Please create a new bug ticket for it as this merge request is already completed. Issues found are new bugs now.

@Krzmbrzl Krzmbrzl deleted the feature-markdown branch November 9, 2022 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client feature-request This issue or PR deals with a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quotes don't italicize in Text Messages Markdown parser for chat
8 participants