Should settings be applied when you open a file (or when you save it)? #248

Open
glen-84 opened this Issue Dec 23, 2015 · 9 comments

Projects

None yet

3 participants

@glen-84
glen-84 commented Dec 23, 2015

For example, when I open a file in Atom that contains CRLF line endings, it changes them to LF based on my editorconfig settings, but I'm not sure if this is the correct behaviour.

Could you please clarify in the documentation whether settings should be applied when opening files or when saving them?

IMHO it should only apply settings when saving a file, as you may just open a file to read it with no intention of making any changes.

I guess it's a bit tricky with the end_of_line setting, because if you start editing, it would insert LFs into a file with CRLFs, and you'd have mixed line endings until the file was saved.

The FAQ entry is not really clear about these settings.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@jedmao
Member
jedmao commented Dec 23, 2015

As the name of this EditorConfig project implies, it changes your editor's settings on file open or tab switch. This means any new lines you add will have the correct line endings, but technically, it shouldn't have to do any kind of transformation to the actual file on file open or file save, because it could inadvertently change the whole file into the configured line endings, which isn't always the desired outcome, especially where file diffs are concerned.

@glen-84
glen-84 commented Dec 23, 2015

@jedmao So am I right in saying that the implementation in atom-editorconfig is incorrect? It should not be doing any full-file transformations?

@jedmao
Member
jedmao commented Dec 23, 2015

My "opinion" has evolved into this mindset about how an EditorConfig plugin should be implemented; however, I have made implementations years ago that violated what I now believe. I know there has definitely been some blurry lines about how a plugin should be supporting EditorConfig and we could definitely benefit from some clarification here, which is probably why atom-editorconfig works the way it does (not a lot of guidance).

That said, yes, I do believe the atom-editorconfig plugin is incorrect in its implementation, but only because there hasn't really been any guidance. I, however, do not make the rules. This is a community and these are my own opinions. I'd be curious to hear what other members of the EditorConfig organization feel about this as well.

If I was to provide rules for plugin authors I would say:

  1. Change editor settings on file open or tabbing over to an already-open file.
  2. Restore settings to their global defaults when a file is closed and between tab switching.
  3. File transformation of any kind shall not be applied on file open, period. Files shall always be in an unmodified/clean state on file open.
  4. The only rules that shall be applied on file save are the following:
    • charset (depending on editor restrictions)
    • trim_trailing_whitespace (see also this proposal)
    • insert_final_newline
@jedmao
Member
jedmao commented Dec 23, 2015

Changes editor settings on file open:

  • indent_style
  • indent_size
  • tab_width
  • end_of_line

Transforms file on file save:

  • charset
  • trim_trailing_whitespace
  • insert_final_newline

Truly, if EditorConfig only tampered with editor settings as a single responsibility, we wouldn't even have file-save transformations, but this is where we are.

@glen-84
glen-84 commented Dec 23, 2015

I agree 100%. I would add that, if one or more of the latter 3 options can be supported via an editor setting (some editors support "save actions"), then that should be the preferred implementation method.

Could we ping some other members for their opinion, or should we just wait for that input?

@jedmao
Member
jedmao commented Dec 23, 2015

It would be extremely nice to publish a guide for plugin developers that clarified all of this. I agree with you that save actions should be the preferred method for supported editors.

@treyhunner @xuhdev what do you think?

@treyhunner
Member

This Wiki page (linked on the bottom of the homepage) is the best we have right now.

@jedmao I agree with your assessment of the "on open" and "on save" settings. Those are ideals/suggestions though since a brand new file doesn't yet have a name or location in some editors.

Feel free to edit/extend that wiki page or suggest something better. 👍

@jedmao
Member
jedmao commented Dec 25, 2015

@treyhunner I think that document is great. Just need to add a section with some specifics (after Christmas). :)

@glen-84
glen-84 commented Dec 30, 2015

For some unknown reason, I've been blocked by @sindresorhus, so I'm unable to submit an issue to atom-editorconfig.

This is what I was going to submit:

Title: The plug-in should not change files when they are opened

Please refer to https://github.com/editorconfig/editorconfig/issues/248.

It should only change editor settings, and *possibly* make changes on file *save*.

Correcting this may also resolve #56, #59, https://github.com/TypeStrong/atom-typescript/issues/731, and others that I'm not aware of.

Please feel free to submit this on my behalf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment