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

Compatibility issues with Highlighting Lines in Code blocks #2544

Open
jaywhj opened this issue Dec 6, 2024 · 5 comments
Open

Compatibility issues with Highlighting Lines in Code blocks #2544

jaywhj opened this issue Dec 6, 2024 · 5 comments
Labels
P: low Low priority bug or request. P: maybe Pending approval of low priority request. T: feature Feature.

Comments

@jaywhj
Copy link

jaywhj commented Dec 6, 2024

Description

I have a paragraph in my markdown text like this, and when I run it, it parses incorrectly, causing the rendered page to be completely misaligned, here's the source file: TestOne.md.

```javascript [4]
function hello(name) {
    let phrase = `Hello, ${name}!`;

    debugger;

    say(phrase);
}
```

It would be rendered like this:

iShot_2024-12-06_14 32 12

This is the correct parsing in Typora:

iShot_2024-12-06_14 32 36


The reason is that the [4] after the token is not recognized, and it treats the [4] as the beginning of the code, which should normally be the next line.

I know this is not the canonical writeup for this project, but it could be designed so that ignore unrecognized markup when it is encountered, thus not affecting the parsing and rendering of later text.This writeup renders fine in other editors, such as Typora, Github and so on.

This writeup is from other project revealjs, I compared, this design is much simpler, just need to set the number, don't need to remember the property name, and it also supports step-by-step highlights.

  • If you agree with this design, you can consider adding support in subsequent updates;
  • If other settings are involved and it is troublesome to modify, then forget it, but it needs to be designed to ignore the markup and not affect the analysis and rendering of the following text;

Here are some write-ups for comparison and reference:

iShot_2024-12-05_12 40 08

Minimal Reproduction

unnecessary

Version(s) & System Info

unnecessary

@jaywhj jaywhj added the T: bug Bug. label Dec 6, 2024
@gir-bot gir-bot added the S: triage Issue needs triage. label Dec 6, 2024
@facelessuser facelessuser added T: feature Feature. and removed T: bug Bug. labels Dec 6, 2024
@facelessuser
Copy link
Owner

I'll start off by saying that while this was filed as a bug, it is not a bug and the docs clearly state the parsing expectations for this extension. I would consider this a feature request or an enhancement at the very least.

Additionally, I will state that this extension is not meant to support every variation of code blocks created in the past, and certainly is not designed with the requirement to adopt all new syntaxes that various parsers may decide upon in the future.

SuperFences was based on the syntax of CodeHilite which has been around probably longer than all the listed examples' implementations. A lot of the examples are probably based on CommonMark which came out much later. Python Markdown is an old-school parser and is not expected to conform to any of the expectations of CommonMark. SuperFences specifically based the syntax on CodeHilite to provide compatibility when people came over from the Python Markdown default provided extension.

It should also be noted that SuperFences provides a generic way to create custom fences to parse unaltered blocks of text. This was mainly done because Python Markdown's implementation of code handling is quite a bit cumbersome due to its need to be implemented as a preprocessor. Providing a generic interface that people could tap into meant others could leverage the work and that I'd never have to rewrite that logic again if I needed to pull off similar logic.

While primarily used for code blocks, it should be noted that the logic is reused for non-code block custom fences. Adding a line highlight specific syntax like [num] would only be used by code highlighting and would be redundant functionality.

Now, regarding the aforementioned example:

```javascript [4]
function hello(name) {
    let phrase = `Hello, ${name}!`;

    debugger;

    say(phrase);
}
```

I think SuperFences logic here is expected and logical. In order to have a fenced code block, the header must be well-formed and conform to expectations. If expectations are not met, the block will be treated as a paragraph, and then the inline code highlighter will be allowed to parse sections within the paragraph, which is exactly what happens. I don't find the output surprising at all. The difference is that GitHub (and possibly some other parsers) will look at the header and, as long as the header doesn't include another `, it will accept all the other junk on the line. This is certainly a choice, but one that I do not agree with.

Is there merit in the new numerical syntax? I will generally say yes. There is certainly nothing wrong with the syntax [num], and I can certainly see it being a subjectively preferred approach, but we do already have a convention, and our aim was compatibility with CodeHilite, and being generic enough so that people could create their own custom fences.

As far as accepting anything in a code fence header, I do not like the idea of code blocks swallowing any and all junk on the fenced code block header line. Having a clear indication when you are providing nonsense to the parser is something that I like, and the parser failing to parse the content as a fence code block is that indicator.

I admit that this is an opinionated viewpoint, but these are, to some extent, opinionated extensions.

@jaywhj
Copy link
Author

jaywhj commented Dec 6, 2024

@facelessuser

Okay, I got it. Thank you for your response, very detailed.

My intention is not to ask you to change a new parsing method for this project, just to ask if it's possible to ignore the parsing of this part of the middle bracket [] so that it doesn't affect the later rendering.

I think it should be easy to handle, for example, ```java [3], [3] is preceded by some space, you can split the line with a space, and then just take the first part as a syntax markup, and ignore anything after that line. Github, Typora, and other parsers seem to work this way.

@facelessuser
Copy link
Owner

My intention is not to ask you to change a new parsing method for this project, just to ask if it's possible to ignore the parsing of this part of the middle bracket [] so that it doesn't affect the later rendering.

I wasn't sure. It seemed to mention it as a possible approach with a fallback to ignoring the content if implementing was not desired. Obviously, this was my misinterpretation 🙂.

I think it should be easy to handle, for example, ```java [3], [3] is preceded by some space, you can split the line with a space, and then just take the first part as a syntax markup, and ignore anything after that line. Github, Typora, and other parsers seem to work this way.

No, I understand this part, but as I detailed earlier, I'm not sure I agree with this approach. These implementations make a number of assumptions. If the line starts with ``` (or more) and has no other ` on the line, then it blindly accepts the entire line as the start of a fenced code block. But this might not be the case as inline code can span multiple lines within a paragraph.

Additionally, as stated earlier, I'm not sure I'm okay with hiding bad syntax on the header line and confusing people by making them think their header is valid, but then the functionality they expected isn't shown in the output. Part of my reasoning is preventing people from filing issues here. If they see the code block not rendered properly, they may take a closer look at their syntax. If this prevents even a handful of filed issues here that I have to respond to, it is worth it.

@jaywhj
Copy link
Author

jaywhj commented Dec 9, 2024

Okay, thank you very much.
On the premise of not affecting the existing functions, it would be nice if it could be compatible with this writing, after all, other mainstream editors can parse and render normally 😁.

@facelessuser facelessuser added P: maybe Pending approval of low priority request. P: low Low priority bug or request. and removed S: triage Issue needs triage. labels Dec 9, 2024
@facelessuser
Copy link
Owner

after all, other mainstream editors can parse and render normally

Yes, specifically a number of CommonMark parsers mainly, but many that are not do not.

it would be nice if it could be compatible with this writing

Sure. I'll say this, if it is something I can make "opt-in", without too much complexity, then I might consider it. I'll leave this as a "maybe" with low priority for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P: low Low priority bug or request. P: maybe Pending approval of low priority request. T: feature Feature.
Projects
None yet
Development

No branches or pull requests

3 participants