Ability to indicate when to automatically show or hide the wrap-guide

[Trigan2025]

Description of the Change

Add the ability for the user to indicate when to automatically show or hide the wrap-guide between always, if wrapping and if wrapping at preferred line length.
This is done by listening the wrapping changes of text-editors with onDidChangeSoftWrapped and of the SoftWrapAtPreferredLineLength setting.

As for the why, I always find it strange that the wrap-guide stays visible when the Soft Wrap At Preferred Line Length setting was disabled because of the Soft Wrap one staying enabled.

I also take the liberty to change the ugly `.${this.editor.getGrammar().scopeName}` to the more readable and simple this.editor.getRootScopeDescriptor() as well as making some parts more async-like (but I admit to not be certain of the pertinence of them all).

Alternate Designs

Initially I was trying to manage that directly in my theme package, but the more I worked on it the more I feels that it belongs more there than in my package.

However, this change mostly add a condition to allow or discard the appending of the guides, we could do differently by adding a class to the guide element indicating whether soft-wrapping is active so that it can then be stilled accordingly directly by the themes. (will just move the condition from updateGuides to appendGuide)

Possible Drawbacks

This is only done by adding one listener for the new setting and two listeners per editor, and they are as short and async as I could made them, so I don’t think it induce any drawbacks.

Verification Process

First of all, I of course update and run the test suite to ensure that all changed functionality works as expected and do not introduce any error and do not break any already existing functionality.

I also test it locally by opening an editor window with some file and played with both global and scoped settings related to soft-wrapping to verify that all behave as expected.

[confused-Techie]

Thanks a ton for contributing! It's really appreciated!

I'll have to take some time to look over these changes (But of course anyone else feel free to do so).
Otherwise, I'll point out the tests are failing in CI for the package wrap-guide, you can see the run and what's failing here (Or otherwise click the "Package tests / Package (wrap-guide) (pull_request)" item in the jobs run below.

One thing I'd just like to be overtly clear about, that sounds like you already gave consideration, but by default the behavior of this package doesn't change at all correct? Just making sure that this new feature is totally opt in.

Again, thanks!

[Trigan2025]

Sorry for the late reply.
And my bad, I only modify and test the wrap-guide-element-spec as the dumb as I am, I will fix that right away.
As for the default behavior, I made the "atPreferredLineLength" the default of the new setting option (so, need to take that in consideration in the spec), but we can re-change it to "always" if you prefer.

[Daeraxa]

As for the default behavior, I made the "atPreferredLineLength" the default of the new setting option

Generally, unless there is a good reason to change it (and I'm not saying this isn't, haven't tried the change out yet) we prefer to not alter the default behaviour, at least not without an opt-in trial period.

[Trigan2025]

So, should I push a new commit now to this change it ?

[confused-Techie]

So, should I push a new commit now to this change it ?

Yeah feel free to do one last push to modify the default behavior of the change. Once that last push then builds binaries I'll be able to test out the changed behavior. So appreciate whenever you're able too. Thanks a ton for working with us here. Otherwise your changes do sound great

[Daeraxa]

I also take the liberty to change the ugly .${this.editor.getGrammar().scopeName} to the more readable and simple this.editor.getRootScopeDescriptor() as well as making some parts more async-like (but I admit to not be certain of the pertinence of them all).

I just want to check this with @pulsar-edit/core, although the scope of this is small we are implementing some changes unrelated to the feature itself. Should these be in a separate PR to the functionality change or is this ok?

[savetheclocktower]

I can't find where this is allowed, since getRootScopeDescriptor returns a ScopeDescriptor and the scopeSelector keyword option of atom.config.set expects a string. Has this been tested?

[Trigan2025]

In the spec file I does the following and it pass without errors:

        const scopeDescriptor = editor.getRootScopeDescriptor();

        atom.config.set('editor.softWrap', false, {scopeSelector: scopeDescriptor});

        expect([atom.config.get('editor.softWrap'),
          atom.config.get('editor.softWrap', {scope: scopeDescriptor})]).toEqual([true, false]);

Not the same config name, but same thing.

[savetheclocktower]

Sure, I don't doubt it works if you pass literally the same object back when you look it up. But if it were being coerced to a string like "[object Object]", that test would pass, and it would be wrong.

Let's change it back just for now and we can figure out what's going on here later.

[Trigan2025]

I also do some more test since, and the config file is update as normal.
I also tried to pull the wire to see why it works and this is what I found:

• atom.config.set call setRawScopedValue
• that call this.scopedSettingsStore.addProperties
scopedSettingsStore being an object from https://www.npmjs.com/package/scoped-property-store
• that will call Selector.create
• that will call slick.parse
• that will do ('' + expression)
• that will implicitly call scopeDescriptor.toString
  That return this.getScopeChain()

However, I am not her to force myself her, so will comply and change it back.

[savetheclocktower]

Should these be in a separate PR to the functionality change or is this ok?

Thanks for calling this out — I'd prefer to revert that specific change, since the meaning of the line is subtly different. Let's hold off on merging that until this is addressed.

[DeeDeeG]

I can confirm this appears to work as intended.

(If you enable the "soft wrap" setting in the editor settings (or via View menu --> "Toggle Soft Wrap"), then the wrap guide shows when you set "when wrapping" in the wrap-guide settings. I think this setting should be changed to say "When soft wrap is enabled", or it should only show when actually wrapping a line.)

(If you enable the "soft wrap at preferred line length" setting and the "soft wrap" setting, both in the editor settings, and set "when wrapping at preferred line length" in wrap-guide settings, then the guide shows. Once again, I think this should be renamed to "when wrapping at preferred line length is enabled", or it should show only when the editor is actually wrapping a line.)

---

On a bit of a tangent:

Since you were wondering why the wrap guide would be shown when soft wrap is disabled: FWIW, showing the wrap guide "always" is mostly useful for people who are manually managing their line lengths. Such as to satisfy linters or style guides, commit message line length requirements, etc...

(I personally never gave much thought to the "soft wrapping" feature, since I usually have soft wrapping disabled, and I personally feel a need to see the real line lengths at all times so I can avoid very long lines of code and just see how the file is laid out on-disk.)

---

Whether to accept this PR:

I am personally neutral but see no reason to object. I have no strong feelings about this feature, but I can say it does seem to essentially work, and I could see why some people might want the guide to reflect the soft wrapping feature, if they're mainly using it to think about the soft wrapping feature, as opposed to using it to manually wrap lines (by typing newlines). (Soft wrapping is just about displaying the files a certain way, not managing line length in the file with actual newline characters.)

I haven't reviewed the actual code, but at a glance it doesn't look too "out there" is all I can say.

EDIT: I will switch my response to 'lite-approve', which is what I do when I haven't reviewed the actual code but am willing to support something after it is merged. I don't anticipate problems with this code, to be honest, but hopefully any problems that would arise wouldn't be too tricky to troubleshoot.

Thank you for the contribution.

[Trigan2025]

Sorry again for late reply and thank you to have take the time to explain about the why we would want the wrap-guide to be shown when not soft-wrapping at the preferred line, its true that I have tendency to forgot that some could sometimes have to wrap manually.

And indeed, I had hesitated and I didn't really know how to formulate the options, I wanted to keep them as short as possible and not make the When repeated, but it makes it a little bit incorrect, so I will apply your suggestion which actually seems more appropriate.

[DeeDeeG]

Thank you for addressing the review feedback, and thanks for the contribution!

I'm going to go ahead and merge this, it'll be in the next Rolling binary as soon as CI finishes after this merges.