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

Implement max_line_length #53

Closed
egamma opened this issue Jun 16, 2016 · 21 comments
Closed

Implement max_line_length #53

egamma opened this issue Jun 16, 2016 · 21 comments

Comments

@egamma
Copy link

egamma commented Jun 16, 2016

From @rdumont on June 15, 2016 21:8

A max_line_length property would be nice to have, as described in https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length.

Copied from original issue: Microsoft/vscode-editorconfig#27

@egamma
Copy link
Author

egamma commented Jun 16, 2016

This version of the extension has been deprecated in favor of https://github.com/editorconfig/editorconfig-vscode. Moving the issue

@jednano
Copy link
Member

jednano commented Jul 13, 2016

@rdumont, how do you suggest this feature be implemented? I don't see a single vscode setting that we could associate w/ max_line_length. True, there are some line wrap settings, but that's not the purpose of a max_line_length. It might be more appropriate for a linter to tap into this setting. What do you think? Any ideas?

@SamVerschueren
Copy link
Collaborator

I'm 👎 on this. I'd suggest using soft line wrapping instead of hard line wrapping as well. But that's just my opinion :).

@jednano
Copy link
Member

jednano commented Jul 13, 2016

Me too, all the way, but this is an official EditorConfig setting, so I'm trying to be accommodating here. Still, I believe it's best achieved w/ a linting tool of some kind. I'm going to close this issue for now as something NOT planned for the roadmap.

@jednano jednano closed this as completed Jul 13, 2016
@alexewerlof
Copy link

I come from Atom (mainly due to VSCode's awesome debugging features) but the lack of this feature is a bit annoying because it relies that I add a linter to my project. Not all projects are big or important enough to justify a linter but all my projects could use the max_line_length setting for softwraps because for example it makes multi-pane editing a pleasure. Since I work with both Atom and VSCode, it'd be great to have a similar experience. It may sound like I'm the minority but implementing this feature may benefit others nevertheless.

See the discussion on Atom editorconfig: sindresorhus/atom-editorconfig#12

PS. this feature recently broke due to some atom update

@SamVerschueren
Copy link
Collaborator

@sindresorhus any opinions on this?

@sindresorhus
Copy link
Member

I don't really see the point of that rule for soft-wrapping, as you can already set it yourself in the editor and it makes no difference to the actual code.

@jednano
Copy link
Member

jednano commented Apr 20, 2017

See also editorconfig/editorconfig#280

@jednano
Copy link
Member

jednano commented Apr 20, 2017

@userpixel, why don't you just use the editor.wordWrap user setting and call it a day?

{
    "editor.wordWrap": "wordWrapColumn",
    "editor.wordWrapColumn": 80
}

This way, you don't have to force the behavior on the entire team, as it doesn't change the source code at all. It's opt-in!

@alexewerlof
Copy link

Re:

as you can already set it yourself in the editor and it makes no difference to the actual code.

That's correct and judging from the fact that pretty much all editorconfig settings have an actual effect on the files (except tab_width) I'd assume we're aiming for a physical line length (not soft wraps). In Atom, there is a vertical guide line that well adjusts to the value of max_line_length. In VSCode it is "editor.rulers": [80]

Re:

why don't you just use the editor.wordWrap user setting and call it a day?

That has been my workaround so far and it works great apart from the fact that I have to manually set it which I guess is against the reason there is an .editorconfig on the first place.
In projects that I share with my team, there's definitely a linter involved which takes care of it.


So to sum it up:

  • Not a big deal, but nice to have
  • No soft wraps but rather the ruler "editor.rulers": [max_line_length]

@rdumont
Copy link

rdumont commented Apr 20, 2017

As stated in editorconfig/editorconfig#280, the definition is indeed not applicable. Not in general at least. When I opened the issue, I was aiming at using max_line_length in Markdown code, which would be extremely helpful. But I suppose this should be a setting for whatever Markdown syntax extension that a person desires to use.

@jednano jednano changed the title Impelment max_line_length Implement max_line_length Apr 20, 2017
@jednano
Copy link
Member

jednano commented Apr 20, 2017

@userpixel not only does soft wrapping not affect the source code, but it's highly subjective as to whether some people like it or not. Myself? I typically work with it off, because I like to go by guidelines. If soft wrap were on, I wouldn't immediately detect when something goes past a guideline.

My point in saying this is that I would be offended if I were on a project that forced me to soft wrap on every file I open; thus, I don't think it's a good idea to implement. We don't want to get in the way of people's opinions.

By the same token, tab_width doesn't affect the source code and if it were up to me, this rule wouldn't even exist either. I would be equally offended if someone forced me to display tabs at 8-space width, for example.

You said it's a nuisance that you have to define soft wrap manually, but what I'm saying is that you should have to define this manually, because it's a subjective setting. Besides, it's a one-time setting and you never have to worry about it again.

In conclusion, subjective rules should be applied manually and in your user settings. This is, of course, my opinion.

@andyearnshaw
Copy link

I know this is an old issue, but there seems to be some kind of misunderstanding here. max_line_length isn't a soft-wrapping rule, it's a hard-wrapping one. Hard wrapping has many desirable benefits, such as:

  • better for print
  • better for command line viewing
  • easier code navigation in some editors
  • better visibility in patches

You said it's a nuisance that you have to define soft wrap manually, but what I'm saying is that you should have to define this manually, because it's a subjective setting. Besides, it's a one-time setting and you never have to worry about it again.

While soft wrap may be subjective, max_line_length is not a subjective setting, it's often enforced as company policy. 100 is the maximum line length defined by AirBnB's JavaScript style guide, for instance, and this is favoured by many development teams across the world.

@jednano
Copy link
Member

jednano commented Feb 9, 2019

@andyearnshaw I don't think anyone's confusing max_line_length with soft wrapping, but there was some back and forth about soft- and hard-wrapping. I can see what that was confusing to read. Anyway, perfectly clear that max_line_length is supposed to be a hard wrap.

As for subjectiveness, I was talking about soft wrapping, not hard.

That said, EC still should not be touching this rule (max_line_length), because it would have to be syntax aware. I think it makes better sense as a lint rule.

@andyearnshaw
Copy link

@jedmao thank you for the clarification.

I agree with you that it should be syntax aware, but not everyone uses linters for everything. Most codebases are made up of more than one language. Looking at one of our repos, we have HTML, JS, CSS, Markdown, JSON and YAML. We don't even use or want to install separate linters for all of those, we don't have much HTML so code reviews are enough to keep things tidy, barely anybody lints Markdown, and it has no syntax issues with hard wrapping.

It could at least be included as an option for certain file types, with a well-explained caveat that it can cause syntax problems if it breaks in the wrong place. Vim managed this by having it baked in, and I don't recall having any problems in the many years I used it.

@jednano
Copy link
Member

jednano commented Feb 10, 2019

If it's an option for certain file types, then it's an option for all file types. That's how EditorConfig works. We can't just support it for one file type.

I'm extremely fearful of breaking code when applying a rule such as this. I'm not comfortable causing syntax problems with or without the well-explained caveat. If this is so important to you, personally, I suggest you fork the repo and add it in a separate extension until the EditorConfig team comes to some sort of agreement as to how to support this rule in a syntax-agnostic way, which brings me to my next point. Please file the issue on the main EditorConfig repo, as this request is not specific to this extension.

@andyearnshaw
Copy link

If it's an option for certain file types, then it's an option for all file types. That's how EditorConfig works. We can't just support it for one file type.

Yes, it would make for an awkward whitelist, anyway.

If this is so important to you, personally, I suggest you fork the repo and add it in a separate extension until the EditorConfig team comes to some sort of agreement as to how to support this rule in a syntax-agnostic way, which brings me to my next point.

My main issue is that I loved this feature of Vim. Since moving to Code, I'm using Rewrap to solve the problem, and that does a pretty good job except that it doesn't read .editorconfig files. If editorconfig-vscode even just had the option to apply rulers based on the max_line_length rule, then Rewrap would handle the rest.

@jednano
Copy link
Member

jednano commented Feb 11, 2019

Now it's getting really complicated. I personally already have 3 rulers setup in my user configuration. If a separate max line length rule were to be specified, would it override all of my rulers or just add another one to it? How then would the editor know which ruler to use for wrapping? I think it makes the most sense, in this case, to submit an issue on the Rewrap extension to read EditorConfig files via the js core, just like this extension does.

@andyearnshaw
Copy link

This is not meant to sound rude, but I think you are overcomplicating it. Something like:

[ ] Add a ruler for max_line_length (you can use an extension like Rewrap for hard wrapping)

This is pretty clear-cut. If I have other rulers, I know that this is going to add one based on the setting in my .editorconfig file.

How then would the editor know which ruler to use for wrapping?

Rewrap can wrap to any of your rulers. You can change the wrapping ruler using a key combination, and that one becomes the default if you have auto-wrapping on.

I think it makes the most sense, in this case, to submit an issue on the Rewrap extension to read EditorConfig files via the js core, just like this extension does.

Isn't this the official EditorConfig plugin for VSCode? Why isn't this the place to come if you want a feature of EditorConfig to be implemented? Yes, I could ask the Rewrap author or submit a PR there, but other people might genuinely find a ruler at max_line_length useful without the wrapping functionality. Again, I'm not trying to be rude or abrupt, I just feel that this would be the best place for some kind of max_line_length functionality.

This isn't a hill that I'm going to die on. It is a minor inconvenience, a feature that I miss from Vim. If I have to manually configure my rulers for different projects, then I'll just put up with it.

@jednano
Copy link
Member

jednano commented Feb 11, 2019

Isn't this the official EditorConfig plugin for VSCode? Why isn't this the place to come if you want a feature of EditorConfig to be implemented?

@andyearnshaw to be perfectly clear, this is not even an official EditorConfig rule yet or maybe ever:

  1. It's nowhere on the editorconfig.org page.
  2. It's defined nowhere in the JavaScript core.
  3. It's nowhere in the plugin guidelines.
  4. It is here, but vaguely could be referenced as a potentially future EditorConfig rule (not current).
  5. It's still being researched.
  6. It has been discussed at great length and with much controversy here and here, without really coming to any resolution.

I maintain my previous declaration that this issue is not specific to this extension. Feel free to continue conversation on whether this rule should even exist on the main EditorConfig issue tracker.

@editorconfig editorconfig locked and limited conversation to collaborators Feb 11, 2019
@jednano
Copy link
Member

jednano commented Feb 11, 2019

See editorconfig/editorconfig#387 for continued conversation around this rule.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants