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

Adding linebreak stripping #351

Merged
merged 2 commits into from
Sep 7, 2023
Merged

Adding linebreak stripping #351

merged 2 commits into from
Sep 7, 2023

Conversation

nyarly
Copy link
Contributor

@nyarly nyarly commented Sep 6, 2023

Adds --strip-newlines which collapses single newlines into a space - effectively what would happen during normal HTML rendering. This is to address Confluence's behavior, which is to insert a <br>.

Closes #92

}
}

func TestCompileMarkdownDropH1(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also added tests for DropH1 - I can't swear to their accuracy, but they look right

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for adding these!

}
}
} else {
_ = w.WriteByte(byte(r.softBreak))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is effectively taken directly from goldmark (like the other renderers), with this exception.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a reference where you took it from as a comment?

<p>two-2</p>
<p>three-1</p>
<p>three-2</p>
<p>space-1 space-2</p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what we want

</ul>
</ac:rich-text-body></ac:structured-macro>
<ul>
<li>Regular list that runs long</li>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the thing that cannot be (easily) got by e.g. a sed script.

import (
"unicode/utf8"

//"github.com/kovetskiy/mark/pkg/mark/stdlib"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you remove the import here if you don't need it?

@mrueg
Copy link
Collaborator

mrueg commented Sep 7, 2023

Thanks for the contribution! I've added a couple of comments to it.

@@ -63,6 +63,13 @@ var flags = []cli.Flag{
Usage: "don't include the first H1 heading in Confluence output.",
EnvVars: []string{"MARK_H1_DROP"},
}),
altsrc.NewBoolFlag(&cli.BoolFlag{
Name: "strip-linebreaks",
Value: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we enable it by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be a change to the output, which is why I set it disabled by default. If it were me, I'd enable by default on the next major release, and remove the option on the release after that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, makes sense, I'll switch it on once we release mark-10.x

reg.Register(ast.KindText, r.renderText)
}

func (r *ConfluenceTextRenderer) renderText(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add a comment on top why this is needed and what it does instead of the default text renderer from goldmark

@nyarly
Copy link
Contributor Author

nyarly commented Sep 7, 2023

I've updated the PR in response to comments.

@mrueg mrueg merged commit f0d7bfd into kovetskiy:master Sep 7, 2023
4 checks passed
@mrueg
Copy link
Collaborator

mrueg commented Sep 7, 2023

Thanks for the feature @nyarly ! While I have you here and you probably also looked a bit deeper into goldmark, any thoughts on how to solve #324 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle semantic line breaks when converting markdown file to an HTML file
2 participants