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

Line breaking on "|" #810

Closed
marcoscaceres opened this issue Apr 3, 2019 · 7 comments
Closed

Line breaking on "|" #810

marcoscaceres opened this issue Apr 3, 2019 · 7 comments

Comments

@marcoscaceres
Copy link
Contributor

Given, for example:

<p>... text.... |details| ... text</p>

Tidy inserts a line break resulting in:

 <p>....|
  details|...
</p>

Which is not great, as it renders as | details|.

Expected: |details| to stay intact.

@marcoscaceres
Copy link
Contributor Author

(happy to help fix this if someone can point me at where this might be happening in the code)

@marcoscaceres
Copy link
Contributor Author

Just to clarify, this only happens if the "|" hits close to the line length. Here is real example where you can see it happening:

          <li>Establish the request's id:
            <ol>
              <li data-tests="payment-request-id-attribute.https.html">If |
              details|.<a data-lt="PaymentDetailsInit.id">id</a> is missing,
              add an <a data-lt="PaymentDetailsInit.id">id</a> member to
              |details| and set its value to a <abbr title=
              "Universally Unique Identifier">UUID</abbr> [[RFC4122]].
              </li>
            </ol>

@geoffmcl
Copy link
Contributor

geoffmcl commented Apr 4, 2019

@marcoscaceres thank you for the observation, and sample input, to repeat the show... default config... but is there an issue here?

It seems libTidy has chosen the | character, as a wrappable point... maybe you do not agree with that... fair enough...but why? ... W3C REC links would help...

Have not yet looked at the code, too much, that makes this wrappable decision, given a text output, wrappable stream... Might be simple to exclude the | char from that... but what other consequences result? ... need to explore, understand, etc... more...

Need more information, to proceed,,, thanks...

@marcoscaceres
Copy link
Contributor Author

marcoscaceres commented Apr 7, 2019

Hi @geoffmcl... both W3C ReSpec and BikeShed are using "|thing|" as meaningful. For example:

https://github.com/w3c/payment-method-basic-card/pull/73/files

but what other consequences result? ... need to explore, understand, etc... more...

Happy to explore more, but it seems to make sense that anything|other thing|more things is semantically distinct from the same containing spaces. Consider even other things like:

  • "foo||bar" is not "foo ||bar" or worse "foo| |bar"
  • "|foo|bar" is not either "| foo|bar" or "| foo| bar" and so on.

There is no valid case I can imagine where dividing on a "|" makes sense as it changes the presentation and potentially the meaning of what the author intended.

@marcoscaceres
Copy link
Contributor Author

Added another case above ("foo| |bar")

@geoffmcl
Copy link
Contributor

geoffmcl commented Apr 8, 2019

@marcoscaceres thank you for the additional feedback, and samples...

I share your concern that some text ...|details|... should not be changed to ...|\ndetails|..., due to tidy wrap...

This would change the presentation, ie the rendering, which tidy tries hard NEVER to do... a rendering change would be a bug in my book...

And potentially change the meaning the author intended...

Maybe I was wrong in my quick assessment that libTidy has chosen the | character, as a wrappable point... seem not true... sorry... still to fully confirm in the code... but...

I have now experimented with your samples, and more, also changing the config... wrap width, indent, etc, etc... and can not repeat the problem!!!

In text like ... foo||bar ..., the block foo||bar is never broken...

Even just foo||bar||foo||bar||foo||bar||foo||bar||... repeated, forever!, in a single line, will be output as one unbroken line... always... no wrap... tidy memory limits apply...

So at least I can not find a working config and sample that demonstrates this problem... and can not fix a problem that can not be repeated... help...

Look forward to further feedback... thanks...

@marcoscaceres
Copy link
Contributor Author

@geoffmcl, I'm tremendously embarrassed. You were correct. What had happened was that I'd done a global search and replace of <var> with |, and I'd neglected to notice that tidy was adding line breaks on <var> (which the browser trims on display).

Anyway, I went through and fixed those and now it's all working 😅 Sorry to have wasted your time.

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

2 participants