Automatically add line-numbers when soft-wrapping (#2404)

[Tim-Siu]

What is the purpose of this pull request?

• Documentation update
• Bug fix
• Feature addition or enhancement
• Code maintenance
• DevOps
• Improve developer experience
• Others, please explain:

Overview of changes:
Currently, code wrapping is supported through the codeBlockWrapButton plugin. A problem with wrapping is that the indention is lost when a line of code is wrapped, which may alter the semantic meaning of the code for certain programming languages (e.g., Python). A workaround is that we automatically add line-numbers for the code when wrapping is applied, regardless of whether it is explicitly specified by the site creator.

Anything you'd like to highlight/discuss:
This PR hasn't addressed the printing part of (#2404) yet.

Testing instructions:
Add a fenced code block with a really long line in the test site. Activate the codeBlockWrapButton plugin by adding

  "plugins": [
    "codeBlockWrapButtons"
  ]

in site.json of the test site.

Proposed commit message: (wrap lines at 72 characters)

---

Checklist: ☑️

• Updated the documentation for feature additions and enhancements
• Added tests for bug fixes or features
• Linked all related issues
• No unrelated changes

---

Reviewer checklist:

Indicate the SEMVER impact of the PR:

• Major (when you make incompatible API changes)
• Minor (when you add functionality in a backward compatible manner)
• Patch (when you make backward compatible bug fixes)

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

• To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

• what was the old feature that was made obsolete
• any replacement feature (if any), and
• how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a265c73) 48.87% compared to head (d9497d6) 48.87%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2413   +/-   ##
=======================================
  Coverage   48.87%   48.87%           
=======================================
  Files         124      124           
  Lines        5238     5238           
  Branches     1109     1109           
=======================================
  Hits         2560     2560           
  Misses       2371     2371           
  Partials      307      307           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[yucheng11122017]

Hmmm I see.. this current implementation adds line numbers regardless of user's settings.
I'm concerned about whether this would be flexible enough for the user
What do @kaixin-hc @EltonGohJH @itsyme think?

[EltonGohJH]

Hmmm I see.. this current implementation adds line numbers regardless of user's settings. I'm concerned about whether this would be flexible enough for the user What do @kaixin-hc @EltonGohJH @itsyme think?

I agreed that it will be inflexible and the users may not expect this behaviour. Maybe we can target the printing aspect?

Wdyt?

@yucheng11122017 @kaixin-hc @itsyme

[kaixin-hc]

It feels like the main issue is what if it wraps line numbers the user does not expect to wrap (since not all languages consider indents the same way as python). If we force this change for all wrapping when clicked regardless of users settings, I'm not sure yet that there would be no undesired behaviour.

If when printing, not wrapping and wrapping but no line numbers both often distort the meaning unexpectedly ... Then I agree with Elton that a change made only for print makes sense? In other words when printing code blocks will auto wrap and add line numbers, otherwise it will follow existing behaviours.

Another alternative (but breaking) might be to have all code blocks automatically have line numbers that must be manually disabled, which would cause the default behaviour to be "correct" when printing if not otherwise overriden. But it's probably not worth making a breaking change for this.

[yucheng11122017]

The current behavior would definitely be a breaking change as well. I would prefer to add only when printing as well
@damithc is that ok?

[damithc]

If when printing, not wrapping and wrapping but no line numbers both often distort the meaning unexpectedly ... Then I agree with Elton that a change made only for print makes sense? In other words when printing code blocks will auto wrap and add line numbers, otherwise it will follow existing behaviours.

Yes, when printing, should be auto-wrapped (but only if there are lines longer than the container width), and line numbers should be added. However, if wrapping is not needed for a given code block, the current printing behavior should remain (i.e., no line numbers unless already enabled by the user), right?

Another alternative (but breaking) might be to have all code blocks automatically have line numbers that must be manually disabled, which would cause the default behaviour to be "correct" when printing if not otherwise overriden. But it's probably not worth making a breaking change for this.

This is what we had in an earlier version but we changed it because we found most of the time code blocks don't need line numbers and it was a hassle to disable it in many places. So, we decided the default to be 'no line numbers'.

It feels like the main issue is what if it wraps line numbers the user does not expect to wrap (since not all languages consider indents the same way as python). If we force this change for all wrapping when clicked regardless of users settings, I'm not sure yet that there would be no undesired behaviour.

The current behavior of the line-wrapping plugin is already flawed because it doesn't differentiate between real line breaks and soft/fake line breaks. Alternatives:

1. Add a new symbol to indicate soft line breaks, like Intellij does it: This seems hard to do, and requires the reader to know the meaning of the new symbol.

2. Show line numbers (even if not enabled) when line wrapping: This seems easier to do, and more intuitive for the reader to understand. It may be unexpected for the reader to see line numbers, but line numbers are harmless, so should be fine?

[kaixin-hc]

Thanks for the additional context prof! Found #1671 where this is discussed and #1734 .

TLDR: agree about softwrapping with line numbers for overflowing code on print (but no change for non overflowing code) makes sense because it helps to avoid changing/losing the meaning of the code, which is the worst part in thr status quo.

Additional rambles on alternative 2:

Alternative 2, which is what I believe this PR is suggesting is followed on several websites. GitHub has line numbers and softwraps all code on mobile; but in the MarkBind context this might not make sense:

Wrapped (Github)	Not wrapped (MarkBind)

Admittedly, in cases like that (an ASCII image that overflows the page size) whether it softwraps on printing or not it will still look weird, and is a rare edge case.

After doing some reading, it seems no line numbers is (frequently) preferred from a UX perspective(discussion 1 (ux exchange), discussion 2 (reddit)), but individuals often have the opposite preference...

So I'm hesitant to say all code wrapped blocks should enable line numbers. If you wrap the code and the meaning doesn't change when you think it's a hard line break instead of a soft line break, I can see why a user who doesn't like line numbers would still want line numbers to be disabled even with code wrapped. Having the numbers go back on would be unexpected (and breaking?)

[itsyme]

Hmmm I see.. this current implementation adds line numbers regardless of user's settings. I'm concerned about whether this would be flexible enough for the user What do @kaixin-hc @EltonGohJH @itsyme think?

I agreed that it will be inflexible and the users may not expect this behaviour. Maybe we can target the printing aspect?

Wdyt?

@yucheng11122017 @kaixin-hc @itsyme

I agree with @EltonGohJH on targeting only the printing aspect as that is the only situation where the user is unable to scroll and the line numbers will improve the clarity in this situation.

Yes, when printing, should be auto-wrapped (but only if there are lines longer than the container width), and line numbers should be added. However, if wrapping is not needed for a given code block, the current printing behavior should remain (i.e., no line numbers unless already enabled by the user), right?

I agree with this behaviour as well

[yucheng11122017]

Hmmm I see.. this current implementation adds line numbers regardless of user's settings. I'm concerned about whether this would be flexible enough for the user What do @kaixin-hc @EltonGohJH @itsyme think?

I agreed that it will be inflexible and the users may not expect this behaviour. Maybe we can target the printing aspect?
Wdyt?
@yucheng11122017 @kaixin-hc @itsyme

I agree with @EltonGohJH on targeting only the printing aspect as that is the only situation where the user is unable to scroll and the line numbers will improve the clarity in this situation.

Yes, when printing, should be auto-wrapped (but only if there are lines longer than the container width), and line numbers should be added. However, if wrapping is not needed for a given code block, the current printing behavior should remain (i.e., no line numbers unless already enabled by the user), right?

I agree with this behaviour as well
Sounds great!

[Tim-Siu]

I cannot think of a way to determine if wrapping happens. Currently, line-numbers are added in markdown-it. Wrapping seems to be handled by css and whether it actually happens is hard to tell. Can anyone suggest any solution to this?

[yucheng11122017]

I cannot think of a way to determine if wrapping happens. Currently, line-numbers are added in markdown-it. Wrapping seems to be handled by css and whether it actually happens is hard to tell. Can anyone suggest any solution to this?

Hi @Tim-Siu if im understanding this correctly, the idea is that line numbers will naturally show if there is line wrapping already? Because if there is wrapping, a new line will be present but the numbers doesn't increase. Please correct me if i'm understanding your question wrongly! :)

[yucheng11122017]

I cannot think of a way to determine if wrapping happens. Currently, line-numbers are added in markdown-it. Wrapping seems to be handled by css and whether it actually happens is hard to tell. Can anyone suggest any solution to this?

Hi @Tim-Siu if im understanding this correctly, the idea is that line numbers will naturally show if there is line wrapping already? Because if there is wrapping, a new line will be present but the numbers doesn't increase. Please correct me if i'm understanding your question wrongly! :)

Hi @Tim-Siu, is there anything you want to clarify? Is there any problem you are encountering?

[Tim-Siu]

I cannot think of a way to determine if wrapping happens. Currently, line-numbers are added in markdown-it. Wrapping seems to be handled by css and whether it actually happens is hard to tell. Can anyone suggest any solution to this?

Hi @Tim-Siu if im understanding this correctly, the idea is that line numbers will naturally show if there is line wrapping already? Because if there is wrapping, a new line will be present but the numbers doesn't increase. Please correct me if i'm understanding your question wrongly! :)

Hi @Tim-Siu, is there anything you want to clarify? Is there any problem you are encountering?

Yes, you understanding of the current implementation in the PR is correct. I haven't figured out ways to implement the suggestions though. I am still investigating.

[yucheng11122017]

Hi @Tim-Siu is this PR still relevant?

[Tim-Siu]

Hi @Tim-Siu is this PR still relevant?

Hi @yucheng11122017 it can be closed if we've decided to only target the printing aspect.

[yucheng11122017]

Closed as no longer relevant - replaced by #2431