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

Break Line Inside Paragraph #167

Closed
gha88 opened this issue Dec 24, 2024 · 11 comments · Fixed by #168
Closed

Break Line Inside Paragraph #167

gha88 opened this issue Dec 24, 2024 · 11 comments · Fixed by #168
Assignees
Labels
enhancement New feature or request

Comments

@gha88
Copy link

gha88 commented Dec 24, 2024

I came across a problem in rendering a paragraph as I wanted because of line breaks.

I'm aware of this link but in my case it cannot solve the problem due to the not known a priori content.
In my case I know I encounter a line break but I don't know how to insert it into the building document that is creating a paragraph.

At the moment I'm unable to render correctly something like this:

`This document describes the structures of RPC API.
The API can be called by the endpoint / RPC.

Each request Response must be compliant with the specification JSON-RPC.`

The first two lines are part of the same paragraph and are rendered as explained before.

Unfortunately due to paragraph block item str method any return carriage is replaced by the split.
The result is something like this:

`This document describes the structures of RPC API. The API can be called by the endpoint / RPC.

Each request Response must be compliant with the specification JSON-RPC.`

In my opinion should be added an inline elements that during the str method will add a special tag that will be replaced after the call

return " ".join(paragraph.split())

in elements.py

What do you think?

@jrg94
Copy link
Member

jrg94 commented Dec 24, 2024

Thanks for the heads up! I will try to take a look at this in a couple days after the holiday. Thanks for all the details.

@jrg94
Copy link
Member

jrg94 commented Jan 1, 2025

Okay, I have some clarifying question(s). Do you want the paragraph to maintain the exact text as-is? If so, you might try using a Raw element. That's sort of the workaround when users don't like the default behavior.

@gha88
Copy link
Author

gha88 commented Jan 1, 2025

I'm aware of the raw element but it's not an option to my case.

Suppose you are building a paragraph maybe from another text format. If so you want to keep modifying the paragraph as the same way you add a link or insert image and so on. The paragraph indeed it's not finished.
Paragraph accept inline element instead raw is a block one and this is not correct.

In my opinion the break line should be an option for inline element.

I don't understand also why the needs to use paragraph split that remove all the return carriage. Why should they being removed in paragraph construction if someone place them in text?

@jrg94
Copy link
Member

jrg94 commented Jan 3, 2025

The main problem is that markdown doesn't seem to be held to any sort of standard. In your example, you want there to be a break in the paragraph, but many tools are not going to respect it. For example, if you drop your exact text into a markdown document on GitHub, the markdown will be rendered without the break.

Markdown
image

GitHub
image

To be fair, I am not opposed to adding a parameter to the Paragraph constructor to allow for respecting the input text as-is. However, in the spirit of Markdown, it's probably better to make use of the Raw block to get the text exactly as you would like it OR split the text into multiple paragraph blocks.

For the record, I try to adhere to the same markdown implementation described here (i.e., John Gruber's), which does seem to support a line break feature that I didn't know existed (which is described here). I replicated it using that Python tool, and it produced the following HTML with the line break you are expecting:

>>> text = """
... `This document describes the structures of RPC API.  
... The API can be called by the endpoint / RPC.
...
... Each request Response must be compliant with the specification JSON-RPC.`
... """
>>> print(markdown.markdown(text))
<p>`This document describes the structures of RPC API.<br />
The API can be called by the endpoint / RPC.</p>
<p>Each request Response must be compliant with the specification JSON-RPC.`</p>

I also replicated this in the GitHub markdown. Just need to add those extra two spaces to the end of the line.

Now, this is tricky to include in the Paragraph directly, so I might entertain the idea of a line break element (kind of like the Inline element but special). That way, you could include it in the middle of your Paragraph as you're building it. On my end, I think it would be equivalent to \n (i.e., space-space-newline). Reading back through this thread, it looks like you mention this idea already.

In my opinion the break line should be an option for inline element.

I might try toying with this idea over the next couple days, but I need to think about how best to implement it. It might be as easy as adding an additional parameter to Inline, but that element is already a bit of a mess.

@jrg94 jrg94 self-assigned this Jan 3, 2025
@jrg94 jrg94 added the enhancement New feature or request label Jan 3, 2025
@jrg94
Copy link
Member

jrg94 commented Jan 3, 2025

Okay, I did a bit of toying around. I'm thinking I might just hardcode the HTML element. After all, the Paragraph object is going to strip the newline anyway. I could potentially remove the stripping, but I worry about what effects that might have on existing codebases.

@jrg94
Copy link
Member

jrg94 commented Jan 3, 2025

Alright, let me know if this is the behavior you wanted:

def test_paragraph_with_linebreak():
    paragraph = Paragraph([
        Inline("First Line", linebreak=True), 
        Inline("Second Line")
    ])
    assert str(paragraph) == "First Line<br>Second Line"

If so, I will update the version and push this out. All other tests still pass.

@gha88
Copy link
Author

gha88 commented Jan 3, 2025

I'm with you about that there is no a standard way to implement the break line.
The most common is the double space plus return carriage. I don't know how much is spread the concept of the html tag break line for html viewer.
Github viewer seems to work correctly.
If you decide for the html tag you should consider these two points based on your test:

  1. The tag should </br> and not <br> because it's a closing tag. Github viewer understand both but it's better to be clear
  2. Inline in this case should accept empty string and of course the option as you did. So I would change the test with 3 Inline elements. The first with a string the second just with line break and the third like the first one.

Another option can be to follow the rule of double space plus return carriage. In this case when you render the Inline content you set a new placeholder that will be replaced after the join paragraph with split.
The placeholder can always be the html tag for example.

Indeed in situation where markdown admit more than one possible way, in my opinion would be very useful a global option settings where the user decide which strategy to use for the final rendering.

@jrg94
Copy link
Member

jrg94 commented Jan 6, 2025

Okay, in the short term, I'm going to keep the html tag version for now. In the long term, I'll have to figure out a workaround/redesign.

@jrg94
Copy link
Member

jrg94 commented Jan 6, 2025

In terms of which tag to use, it seems hotly debated. I'll see it to whatever your preference is: https://stackoverflow.com/questions/1946426/html-5-is-it-br-br-or-br. My preference is for <br /> to mirror the output of the python markdown library.

@jrg94 jrg94 linked a pull request Jan 6, 2025 that will close this issue
@gha88
Copy link
Author

gha88 commented Jan 6, 2025

About the tag I think it's a good choice.

Consider that allowing the final user with a feature like global options settings you could ignore all of this kind of doubts delegating any further customization.
Would you prefer I open a new issue for this?

@jrg94
Copy link
Member

jrg94 commented Jan 6, 2025

Yeah, I think that would be great! Something I can chip away at over time.

@jrg94 jrg94 closed this as completed in #168 Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants