sindresorhus / atom-editorconfig

Helps developers maintain consistent coding styles between different editors
https://atom.io/packages/editorconfig
MIT License
812 stars 80 forks source link

Disabling "Soft Wrap" no longer has an effect #133

Closed papandreou closed 7 years ago

papandreou commented 7 years ago

I just upgraded to version 2.0.1 of the editorconfig plugin, and now my "Soft Wrap" setting (disabled) is no longer being honored. Every time I open or switch to a file in a project that has a max_line_length setting in the editorconfig, the Soft Wrap feature is turned on.

Involved .editorconfig-files

root = true

[*]
end_of_line = lf
charset = utf-8
insert_final_newline = true
trim_trailing_whitespace = true
indent_style = space
indent_size = 4
max_line_length = 80

Directory structure

(huge and irrelevant for this issue)

Installed packages

# This file is for unifying the coding style for different editors and IDEs
# editorconfig.org

root = true

[*]
end_of_line = lf
charset = utf-8
insert_final_newline = true
trim_trailing_whitespace = true
indent_style = space
indent_size = 4
max_line_length = 80
[assetgraph-builder]$ atom .
[assetgraph-builder]$ cat .^C
[assetgraph-builder]$ apm list
Built-in Atom Packages (87)
├── atom-dark-syntax@0.27.0
├── atom-dark-ui@0.52.0
├── atom-light-syntax@0.28.0
├── atom-light-ui@0.44.0
├── base16-tomorrow-dark-theme@1.2.0
├── base16-tomorrow-light-theme@1.2.0
├── one-dark-ui@1.6.0
├── one-light-ui@1.6.0
├── one-dark-syntax@1.3.0
├── one-light-syntax@1.3.0
├── solarized-dark-syntax@1.0.2
├── solarized-light-syntax@1.0.2
├── about@1.7.0
├── archive-view@0.61.1
├── autocomplete-atom-api@0.10.0
├── autocomplete-css@0.11.2
├── autocomplete-html@0.7.2
├── autocomplete-plus@2.31.1
├── autocomplete-snippets@1.11.0
├── autoflow@0.27.0
├── autosave@0.23.1
├── background-tips@0.26.1
├── bookmarks@0.42.0
├── bracket-matcher@0.82.1
├── command-palette@0.38.0
├── deprecation-cop@0.54.1
├── dev-live-reload@0.47.0
├── encoding-selector@0.22.0
├── find-and-replace@0.201.1
├── fuzzy-finder@1.4.0
├── git-diff@1.1.0
├── go-to-line@0.31.0
├── grammar-selector@0.48.2
├── image-view@0.59.0
├── incompatible-packages@0.26.1
├── keybinding-resolver@0.35.0
├── line-ending-selector@0.5.0
├── link@0.31.1
├── markdown-preview@0.158.0
├── notifications@0.65.1
├── open-on-github@1.2.0
├── package-generator@1.0.0
├── settings-view@0.242.2-hotfix1
├── snippets@1.0.2
├── spell-check@0.68.2
├── status-bar@1.4.1
├── styleguide@0.47.0
├── symbols-view@0.113.1
├── tabs@0.101.0
├── timecop@0.33.2
├── tree-view@0.209.3
├── update-package-dependencies@0.10.0
├── welcome@0.35.1
├── whitespace@0.33.0
├── wrap-guide@0.38.2
├── language-c@0.52.1
├── language-clojure@0.21.0
├── language-coffee-script@0.47.2
├── language-csharp@0.12.1
├── language-css@0.37.1
├── language-gfm@0.88.0
├── language-git@0.15.0
├── language-go@0.42.1
├── language-html@0.45.1
├── language-hyperlink@0.16.0
├── language-java@0.23.0
├── language-javascript@0.119.0
├── language-json@0.18.2
├── language-less@0.29.5
├── language-make@0.22.2
├── language-mustache@0.13.0
├── language-objective-c@0.15.1
├── language-perl@0.35.0
├── language-php@0.37.2
├── language-property-list@0.8.0
├── language-python@0.45.0
├── language-ruby@0.69.0
├── language-ruby-on-rails@0.25.0
├── language-sass@0.56.0
├── language-shellscript@0.22.4
├── language-source@0.9.0
├── language-sql@0.23.0
├── language-text@0.7.1
├── language-todo@0.28.0
├── language-toml@0.18.0
├── language-xml@0.34.9
└── language-yaml@0.26.0

Community Packages (13) /home/andreas/.atom/packages
├── atom-beautify@0.29.13
├── atom-wallaby@1.0.12
├── ctrl-dir-scroll@0.2.2
├── editorconfig@2.0.1
├── hyperclick@0.0.39
├── js-hyperclick@1.9.0
├── jshint@1.8.6
├── language-cjson@0.0.1
├── language-terraform@0.7.4
├── linter@1.11.18
├── linter-eslint@8.0.0
├── merge-conflicts@1.4.4
└── mocha-fold@0.1.1

└── (empty)
florianb commented 7 years ago

Hi @papandreou -- thanks for getting in touch. I don't know if i get you right.

You disabled "SoftWraps" in your AtomEditor-Config and therefore expect entirely no Softwraps in your project?

The max_line_length feature was introduced in v2.0.0. And due to the nonexistent possibility of Atom to make HardWraps, it is implemented as SoftWrap. It might be, that what you're facing is the actually desired behavior.

I would like to ask what would you expect how max_line_length is working?

papandreou commented 7 years ago

Thanks for replying :)

What I've disabled is the editorconfig plugin's own "Soft Wrap" setting, which is described as:

Wraps lines that exceed the width of the window. When Soft Wrap At Preferred Line Length is set, it will wrap to the number of characters defined by the Preferred Line Length setting.

I expected that turning off that setting would cause editorconfig 2.0.0+ not to touch the soft wrap setting at all, regardless of whether max_line_length is set in .editorconfig.

Here's how I've set up the editorconfig plugin:

softwrapsetting

florianb commented 7 years ago

I see.. thanks for clarification. These settings from above take control over the .editorconfig Grammar, which means they're applied when working on .editorconfig-files and they are applied by the editors grammar-scope aware packages (like the whitespace-plugin f.e.).

That said to explicitly disable the max_line_length-property of editorconfig when working in your project, you either need to remove the max_line_length-property or set it to max_line_length = off.

papandreou commented 7 years ago

I'm fine with max_line_length having some kind of effect, such as showing that vertical line at the configured limit. As long as it doesn't turn on soft wrap (without allowing me to disable that particular behavior globally).

manuelbieh commented 7 years ago

Same problem here. Code wraps at max_line_length no matter what my Soft Wrap settings say 😕

florianb commented 7 years ago

@manuelbieh -- as described before, if an editorconfig containing a max_line_length-property is applied, the Wrap-behavior for the affected file is skipped.

I treat that behavior as standard-conform (except that we're not hard-wrapping, as the Editorconfig-standard defines.

This issue has been discussed on https://github.com/editorconfig/editorconfig/issues/89.

In order to change that i see the following options:

  1. you can explain why the current implementation of atom-editorconfig behaves different than the standard describes.
  2. you're carrying this to the editorconfig-repository and encourage a change of the max_line_length-definition.
  3. you disable the max-line-length-property.
florianb commented 7 years ago

I close this issue because this seems to be no bug. Feel free to keep discussing.

papandreou commented 7 years ago

I got rid of my problem by removing the max_line_length in the particular project, as it wasn't being adhered to anyway (otherwise I wouldn't have been annoyed by the soft wrapping :).

However, I still think that this plugin touching the soft wrap settings of a given file is a bit too surprising -- especially when there's apparently no way to turn off the behavior. However, it's your software, and right now I'm not so affected by the issue that I'll consider forking it. You'll probably get more reports about this, and then you might reconsider :)

apbard commented 7 years ago

Same problem here and I agree with @papandreou and @manuelbieh. @florianb: couldn't Soft wrap be applied only if set in the global settings or maybe in an option of the editorconfig package?

florianb commented 7 years ago

Probably i don't get it. Could anybody explain to me:

  1. What do you expect max_line_length to do?
  2. How does the EditorConfig-Standard differ from what is implemented in this editorconfig-package?
apbard commented 7 years ago

I would like to be able to specify a "preferred max length" so that the wrap-guide will be able to show the vertical line where I should break the line without enabling the soft/hard wrap at that value. Before the update this was the behaviour I experienced. At the same time, however, I see your point... the new behaviour is what is described in the editorconfig standard you linked.

papandreou commented 7 years ago
  1. Display a vertical line as a guide, maybe highlight the characters that exceed the limit with a red background, like how you're gently guided to write shorter git commit messages.
  2. The spec says "Forces hard line wrapping after the amount of characters specified", and this package chooses to map that to the "Soft Wrap" feature. Those two are not exactly the same -- hard line wrapping would mean forcibly adding a physical line break, where as soft wrapping only affects how the file is viewed within the editor. However, I don't think this plugin should force adding a line break -- that would also be too invasive and suprising, in my opinion.

I took a quick glance at the Emacs plugin, which seems to handle max_line_length by setting the "fill column", which controls where the fill commands wrap (when explicitly invoked).

The vim editorconfig plugin looks like it just highlights the extraneous characters like I suggest: https://github.com/editorconfig/editorconfig-vim/blob/master/plugin/editorconfig.vim#L565-L589

florianb commented 7 years ago

@apbard - thanks for replying that fast. In fact prior v2 of this package, max_line_length was not implemented (and therefore did nothing).

You might consider removing (or disabling) the max_line_length-setting to restore the max_line_length-behavior of < v2.

@papandreou, I appreciate your references to the VIM- and the Emacs-integration and i am glad you came to the same conclusion that it's more convenient to apply soft over hard wraps.

However, being compared to non-conforming implementations misses in my opinion the point. Personally i would probably be fine changing this behavior to a visual hint. But you already mentioned it, applying editorconfig, standardizing the behavior of editors deals at its core with the problem of surprise when multiple environments work on the same resources.

So if i decide to make use of editorconfig, i would research which properties to apply, expecting the described behavior in the standard. The standard stipulates we should (hard) wrap at the given line-length. If we would change the behavior for atom-editorconfig i would have to deal with the (reasonable) complaints about its surprising and non-conformant behavior.

I would like to see this discussion to happen on https://github.com/editorconfig/editorconfig because this is where it belongs to. And if the standard changes we will of course change it, too.

In the meantime i see no applicable changes to the default-behavior which are assumingly not surprising the majority of our user-base. I, however, could live with supporting an alternative (optional) change of behavior, but this would need introducing a persistent config-option and in the history of this plugin this is not usual to quickly introduce any configuration-options.

I would be happy to hear any additional opinions on this - or even get an idea how many users deal with this "problem".

/cc @sindresorhus @slang800 @eddiemonge

P.s.: This is not my software. I am maintaining atom-editorconfig because i want to use it.

sindresorhus commented 7 years ago

I would personally not have implemented max_line_length at all, as I don't see the point of hard-wrapping ever, but I'm not gonna start that kind of flamewar here. We should follow the EditorConfig spec. If you disagree with the spec, take the discussion to https://github.com/editorconfig/editorconfig/issues

manuelbieh commented 7 years ago

If the Atom Plugin API really does not support to force a hard wrap as I read it somewhere above, the editorconfig package is also not following the specs by just implementing a soft wrap instead - so why implement it at all instead of letting the user still "override" the (new) EditorConfig behavior via SoftWrap setting?

Why not make it optional or dependent on the user's SoftWrap setting?

If a max_line_length is specified in the user's .editorconfig and softwrap is enabled in Atom's settings, the package should/could soft wrap at the given max-length (instead of the preferred line length).

If a max_line_length is specified in .editorconfig but softwrap is disabled in Atom, the package should just ignore the max-length since it can't add hard wraps (as defined in the specs) anyway.

It should in any case (no matter if soft wrap is enabled or not) show a length marker at max_line_length.

florianb commented 7 years ago

Since a raising number of users complain about this implementation i am probably going to change the behavior.

Even though i don't understand why people set a max_line_length if they don't want it being applied.

You're welcome to join the discussion for enhancement in #143. ☕️

papandreou commented 7 years ago

Since a raising number of users complain about this implementation i am probably going to change the behavior.

Great to hear that :)

Even though i don't understand why people set a max_line_length if they don't want it being applied.

I think the complaints can be summarized as "there are good and a bad ways to apply it, and mapping it to Atom's Soft Wrap is one of the bad ones if it's turned off in the user's settings".

Also, note that an important use case is working on a fork of someone else's project containing an .editorconfig that you don't want to edit if you can avoid it. If they've put max_line_length in there, it's nice to get some help adhering to it -- without your editor changing its behavior fundamentally.

florianb commented 7 years ago

I agree, this is a reasonable argument.

robcrowther commented 4 years ago

Soft wrap no longer works anymore.

florianb commented 4 years ago

@robcrowther - thank you for your contribution. It is unlikely that your observed behavior is connected to this issue due to code-changes. I'd recommend you to open a new issue instead of following up on this one.