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

PipeTable invalid rendering #744

Open
xeo545x39 opened this issue Oct 3, 2023 · 13 comments
Open

PipeTable invalid rendering #744

xeo545x39 opened this issue Oct 3, 2023 · 13 comments

Comments

@xeo545x39
Copy link

Hi,

Given table is rendered as below.

var content = File.ReadAllText("source.md");

var pipeline = new MarkdownPipelineBuilder()
    .UseAdvancedExtensions()
    .UseGridTables()
    .UsePipeTables()
    .EnableTrackTrivia()
    .Build();

var document = Markdown.Parse(content, pipeline);

using var sw = new StringWriter();
var renderer = new RoundtripRenderer(sw);
var output = renderer.Render(document).ToString();

File.WriteAllText("modified.md", output);
| Column 1 | Column 2 |
|----------|----------|
| Test     | Test 2   |
| Test     | Test 2   |
Column 1Column 2TestTest 2TestTest 2
@pburndorfer
Copy link

pburndorfer commented Oct 5, 2023

Facing the same problem on Windows. Could make a workaround on saving markdown documents in "LF" instead of Windows default "CRLF". Therefore seems to be a line-ending problem on Windows which markdig isn't capable of.
In my code I replace CRLF line-endings with LF.

@xeo545x39
Copy link
Author

Facing the same problem on Windows. Could make a workaround on saving markdown documents in "LF" instead of Windows default "CRLF". Therefore seems to be a line-ending problem on Windows which markdig isn't capable of. In my code I replace CRLF line-endings with LF.

At which moment do you do that? I was trying to save the original file LF like you said as below, but didnt help.

image

@pburndorfer
Copy link

Facing the same problem on Windows. Could make a workaround on saving markdown documents in "LF" instead of Windows default "CRLF". Therefore seems to be a line-ending problem on Windows which markdig isn't capable of. In my code I replace CRLF line-endings with LF.

At which moment do you do that? I was trying to save the original file LF like you said as below, but didnt help.

image

For me during testing it worked saving the file as LF via Visual Studio Code.
In our tool that uses markdig we at first read from the file and directly after that replace the CRLF with LF as follows:
var markdown = File.ReadAllText(file.SourceAbsolutePath).Replace("\r\n", "\n");

That's our workaround currently to be able to use pipetables.

@xeo545x39
Copy link
Author

xeo545x39 commented Oct 5, 2023

What version of Markdig do you use and what is a target of build/configuration on which you run?
I have Windows 10, AnyCPU, Markdig 0.33.0 and trick with string replace does not work unfortunately :(

@pburndorfer
Copy link

Same applies to me, using markdig 0.33.0, running Windows 11.
Maybe have a look at our repo? We use it in the Converter file where we load the raw file and apply some custom processors on it. See line 61.

@MihaZupan
Copy link
Collaborator

If you're creating a renderer yourself, you must configure it with the pipeline you're using.

 var renderer = new RoundtripRenderer(sw);
+pipeline.Setup(renderer);

Otherwise, the renderer won't know what to do with the syntax objects describing the table.

Also, note that EnableTrackTrivia / RoundtripRenderer do not support custom extensions yet (such as tables). They only work with the basic Markdown spec.

@MihaZupan
Copy link
Collaborator

@pburndorfer can you please open a new issue describing the problems you are running into w.r.t. new lines?
From the Converter source you linked to, you're using the regular HTML renderer, so it's unlikely the same issue.

@xeo545x39
Copy link
Author

I see that you render it into HTML, not back to Markdown.
var html = Markdown.ToHtml(markdown, pipeline); Yes, it works. Unfortunately I need Markdown back again. Seems Roundtrip renderer does not work correctly.
Thanks @pburndorfer for additional info.

@xeo545x39
Copy link
Author

@MihaZupan
Ok, so tables aren't supported in render yet. I think it should be a new feature implemented. Tables are really basics.

@MihaZupan
Copy link
Collaborator

The parsing of tables isn't that basic though.
Similar to #155 for the NormalizeRenderer: PR Welcome :)

@xoofx
Copy link
Owner

xoofx commented Oct 5, 2023

To clarify what @MihaZupan explained, there was a tentative of writing a NormalizeRenderer several years ago that was stopped (due to lack of motivation/personal incentive, help from the community...etc.).

Then someone made a large PR #481 that brought roundtrip parsing and renderer. It is a lot more complicated to bring such feature and the original PR made it "only" for the core part of CommonMark.

Most of the extensions are not supported by roundtrip. Implementing support for them can be a significant amount of work (and can be quite challenging technically), so if you need such feature, expect that you will have to bring that support and it will take lots of time.

@xeo545x39
Copy link
Author

@xoofx @MihaZupan
Could you give me permission to create a branch on remote? I receive 403. I wanted to make a PR with draft of the feature.

@xoofx
Copy link
Owner

xoofx commented Oct 6, 2023

Could you give me permission to create a branch on remote? I receive 403. I wanted to make a PR with draft of the feature.

That's not how usually contributors interact with GitHub projects. You need to create a fork of this repo, create a branch in your fork, create/push commits and then once you are ready you can open a pull request.

Repository owner deleted a comment Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants