Enforce non-wrapping (#317)

[mrmanc]

trafficstars

Description

I’ve added configuration to MD01~2~3 to switch it to enforce not wrapping to allow us to enforce a consistent approach. This is described in https://cirosantilli.com/markdown-style-guide/#option-wrap-no

I’m not a ruby dev but this seems to work, but please let me know where I’ve not followed convention or where there are tricks to make things read better.

I’ve created the test cases I can think of. I’m not happy with adding to overlines as these are not lines that are ‘over’. Perhaps this should be renamed something line violation_lines?

I also think there’s a fair bit of duplication in this function now, and some in the file in general around extracting line numbers for various things. I tried extracting a helper function in the test but this didn’t work. I was wondering about adding some more functionality to doc.rb but wanted to check with others first.

Related Issues

Fixes #317.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation (non-breaking change that does not add functionality but updates documentation)
• [ ] Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

• [x] I have read the CONTRIBUTING document.
• [x] Wrote good commit messages
• [x] Feature branch is up-to-date with master, if not - rebase it
• [x] Added tests for all new/changed functionality, including tests for positive and negative scenarios
• [x] The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences

[jaymzh]

huh, I dunno why travis never kicked in. Since I'm going to squash your commits together anyway, could you squash and forcepush to see if travis will kick in?

[mrmanc]

Please don’t merge—I’m going to continue work in this PR to refine the new style option and add further styles, as per DavidAnson/markdownlint/issues/298.

My rough plan is to follow these steps as separate commits

• [x] change existing wrapped to strict
• [ ] change not_wrapped to single
• [ ] add stern option as per the NodeJS version
• [ ] continue to add other options as per DavidAnson/markdownlint/issues/298 (including sentence) one per commit

Following this, I hope to add to the NodeJS version to make the two rules consistent.

[jaymzh]

I dislike "strict" and "stern" - how is a mere mortal supposed to tell these apart? Without reference the docs I'll never remember which is which and I was involved in this PR. I would argue:

• wrapped
• sentence
• not_wrapped

Are the right names. If we want to make aliases to have compat with the NodeJS version that's fine, but lets make these options less opaque.

[mrmanc]

Are the right names. If we want to make aliases to have compat with the NodeJS version that's fine, but lets make these options less opaque.

Sure, that’s a nice compromise—I’ll look at changing this to support the NodeJS options compatible.

@DavidAnson Hope you don’t mind me tagging you in, but I’d rather discuss this in one place as a group rather than going back and forth 😃 Does this suit you—having values such as wrapped and not_wrapped with aliases to ensure compatibility with your version? I imagine you care about the compatibility in both directions. There could be aliases either side, or we could try to find one canonical set of values (and make sure things are backwards compatible).

[jonasbn]

@mrmanc you can set a PR as draft, then it cannot be merged by accident.

See the blog post on the topic.

[DavidAnson]

Per my comment, I think there are at least six different modes this rule can operate in. My proposed names are not perfect and I’m happy to discuss others or alias them as suggested! That said, I’d like to understand how the three proposed names above map to the six modes I list and the current functioning of this rule in Ruby. Which specific behaviors are being suggested?

[DavidAnson]

Related reading:

• History of strict
• History of stern

[mrmanc]

@mrmanc you can set a PR as draft, then it cannot be merged by accident.

Thanks—good advice, but (unless I’ve been missing something) annoyingly you can only do it at the point of creation. I didn’t intend this to be as long-running as it is, so I didn’t create it that way. I’d love a way to retrospectively mark a PR as draft! 😄

[mrmanc]

I’d like to understand how the three proposed names above map to the six modes I list and the current functioning of this rule in Ruby

(comment removed as I later noticed https://github.com/DavidAnson/markdownlint/issues/298#issuecomment-654438412, and will digest that tomorrow as I’m signing off for tonight).

[jaymzh]

Gah I put this response on the wrong ticket. Moving here

Sorry missed that comment

• space (default, first space after 80) - I don't think this should be a thing at all. Either you want it to fit on a terminal screen at 80 chars or you don't.
• strict (nothing beyond 80) - Sure, I'd call this wrapped
• stern (only beyond 80 if no spaces) - Probably shouldn't be a separate mode, but instead an option like exempt_single_word => true - should default to true. That said - is there every any reason NOT to allow this? I can't imagine, but sure, we can make it optional. But it's an orthogonal option.
• single (no wrapping, single line - NEW) - yup, I'd call not_wrapped
• sentence (break after period, etc - NEW) - yup
• semantic (https://sembr.org/, UNSUPPORTED for now) - yup.

So in other words, I'd have wrapped, not_wrapped, sentence, semantic as modes, with a separate option for exempt_single_word.

As to semantic, I guess the only real rules we can enforce there is that you always break after a sentence (just like with sentence) but we allow breaks inbetween sentences.... we certainly can't know where semantic breaks should be inside a sentence.

[DavidAnson]

What I think you’ll find is that the first option, what I called “space” and what you don’t want at all is the default behavior in the Ruby (and therefore Node) implementation.

[jaymzh]

You are absolutely right. That... I've literally NEVER seen a linter do that. That's such a strange implementation.

Well we can't break the default behavior without a major version, so instead lets do this.

Modes:

• space - First space past 80 chars. Current default. Plan to deprecate.
• wrapped - 80 chars, like any other linter. aliased to strict for NodeJs compat. Maybe to be the future default.
• not_wrapped - No wrapping - every para is a single line.
• sentence - Must break at every sentence, and no mid-sentence breaks
• semantic - Must break at every sentence, allows mid-sentence breaks.

Other options:

• code_blocks - same as today, enforce on codeblocks
• tables - same as today, enforce on tables
• exempt_single_words - Exempt single worlds.

?

[waldyrious]

@mrmanc you can set a PR as draft, then it cannot be merged by accident.

Thanks—good advice, but (unless I’ve been missing something) annoyingly you can only do it at the point of creation. [...] I’d love a way to retrospectively mark a PR as draft! 😄

That is actually possible now :)

[mrmanc]

That is actually possible now :)

Amazing! Thanks for the pointer @waldyrious—I totally hadn’t spotted that change creep in. I work with an on-prem GH Enterprise Server most of the time, and so I suffer feature latency.

[mrmanc]

Just a note to say I appreciate it’s been a while, but I haven’t forgotten or given up on this. I still plan to pick this up when I get some time to write code (which I am looking forward to).

[jaymzh]

hey @mrmanc - ping?

[mrmanc]

Argh… thanks for the ping. Sorry for the radio silence. Although I haven’t been able to find time to focus on this, I still want to get it done… and hope to be able to find time soon.

[stephencweiss]

@mrmanc did this die on the vine?

[mrmanc]

@stephencweiss Yes, pretty much. I have tried to get started a couple of times and ended up getting distracted with Ruby version installation issues. Happy for someone else to pick it up, but I’m not ruling out getting around to it.

[jaymzh]

@mrmanc - I recommend using rbenv or rvm to make virtual ruby installs rather than mucking with ruby installs directly.

[mrmanc]

Thanks @jaymzh—I was using rbenv in this case… it was using ruby-install I think that I had the issues.

[jaymzh]

ping, @mrmanc - just checking in again

[jaymzh]

ping again @mrmanc

[jaymzh]

Closing over a year since last activity.