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

Temporarily Set Configuration For Save Operation #65663

Open
jednano opened this issue Dec 25, 2018 · 16 comments
Open

Temporarily Set Configuration For Save Operation #65663

jednano opened this issue Dec 25, 2018 · 16 comments
Labels
api feature-request Request for new features or functionality
Milestone

Comments

@jednano
Copy link

jednano commented Dec 25, 2018

I'm maintaining the EditorConfig extension and am in dire need of a feature that would eliminate some issues with respect to the following settings and potentially more in the future (e.g., end of line setting):

  • files.insertFinalNewline
  • files.trimFinalNewlines
  • files.trimTrailingWhitespace

Specifically, I need a way to short-circuit / override these settings temporarily for a file-save operation, because allowing vscode to perform any kind of trimming is destructive and either irreversible or extremely hacky to fix. It's like asking me to unpunch someone.

Basically, I need a way to change the way that vscode does a built-in save operation on a per-file basis (which is how EditorConfig works). Pre-save TextEdits are not good enough, because whatever I do before a save could potentially be wiped out with vscode's destructive trimming after the fact.

Another option, of course, would be to make EditorConfig a built-in feature of vscode, which I think makes all the sense in the world.

Note: these issues only surfaced after vscode introduced these editor settings, because now we have 2 things trying to do the same thing.

Related issues

@segevfiner
Copy link
Contributor

VS Code already has TextEditorOptions which holds similar settings to be overridden by an extension.

I guess it can be extended with tri-state properties (null - use config, true - run, ignoring config, false - don't run, ignoring config) that allow the extension to override whether these built-in pre save actions will be ran.

This will also allow the extension to reuse the built-in actions rather than reimplementing them in it's own code.

@bpasero bpasero added feature-request Request for new features or functionality api labels Dec 28, 2018
@bpasero bpasero removed their assignment Dec 28, 2018
@Tyriar
Copy link
Member

Tyriar commented Jan 6, 2019

@bpasero any insights into the real solution would be appreciated, I was convinced this was a bug in VS Code itself until I confirmed --disable-extensions worked and searched for the culprit extension. Editor config is a very popular extension (even featured right now) and this bug is forcing me to uninstall.

@bpasero
Copy link
Member

bpasero commented Jan 7, 2019

Adding @jrieken to this thread who added the API for doing something pre-save.

Another option, of course, would be to make EditorConfig a built-in feature of vscode, which I think makes all the sense in the world.

Yes, probably a good idea at some point.

@Tyriar
Copy link
Member

Tyriar commented Jan 7, 2019

There's also this discussion happening on the extension repo: editorconfig/editorconfig-vscode#208 (comment)

@Tyriar
Copy link
Member

Tyriar commented Jan 8, 2019

I think this is the underlying cause of the issue #66214

@Tyriar
Copy link
Member

Tyriar commented Jan 11, 2019

@jedmao is this still relevant given #66214?

@segevfiner
Copy link
Contributor

This issue is about not being able to override per TextEditor whether the pre-save operations run. Which is what the editorconfig extension needs in order to be implemented correctly. Otherwise if the pre-save operations are configured to run via the settings, they will run even if the editorconfig file says otherwise.

@Tyriar what you mentioned is a different issue.

@jednano
Copy link
Author

jednano commented Jan 11, 2019

@segevfiner is correct. This issue is absolutely still relevant.

@jednano
Copy link
Author

jednano commented Jul 27, 2019

Any updates on this guy?

@jrieken jrieken added this to the Backlog milestone Oct 28, 2019
@segevfiner
Copy link
Contributor

Looks like the VS Code project itself is now recommending the EditorConfig extension. Would be nice if it was prioritized adding/changing what's needed so it can be implemented fully and accurately.

@nrayburn-tech
Copy link
Contributor

nrayburn-tech commented Jun 2, 2021

@jrieken, sharing my thoughts on a solution for this. Please correct me if I am wrong anywhere, or you disagree.

My understanding of the problem is that an extension may have a configuration to enforce some coding style that directly conflicts with the vscode save participant settings.
For example, eslint has eol-last, editorconfig has insert_final_new_line, and vscode has files.insertFinalNewLine all of which can conflict in one way or another. In the case of eslint and editorconfig, these setting values can be different for each file in a folder, whereas the files.insertFinalNewLine is applied to the entire workspace.
Eslint modifications are run using editor.codeActionsOnSave.source.fixAll, editorconfig modifications are done with TextDocumentWillSaveEvent.waitUntil with the transformations. In both of these cases, the insertFinalNewLine save participant will be the final change applied.

Your question from #124945 (comment).

Why should an extension prevent my format-on-save logic?

The primary reason for this is that eslint and editorconfig can have a more fine-grained config, whereas the vscode save participants are per workspace.

Proposed Solutions

  1. Original solution, allow an extension to disable any save participants by id. This gives the extension too much control over the save participants. This isn't an acceptable solution.

  2. Solution 2, modify the save participant values from just true/false to be true/false/true with extension overrides/false with extension overrides (possibly specify extensions by id that can override, so that not all extensions have access.) This would work for both the modification methods that eslint and editorconfig use.

  3. Solution 3, modify the order of the save participants so that the editor.codeActionsOnSave are always run last, so that the extensions have the final say about transformations. One downside to this is it only works for extensions using codeActionsOnSave, it wouldn't work for the method that editorconfig uses. Of course extensions could be updated to use whatever is necessary.

In each of the above solutions, the extension would need a way to pass the information in per model. I mention eslint and editorconfig because they are what I am familiar with, but I am sure there are other vscode linting/formatting extensions.

Does solution 2 or 3 work for you? Your thoughts on a different solution?

@jrieken
Copy link
Member

jrieken commented Jun 2, 2021

IMO this is something that needs to be tackled by configuration and not by code. If users install two extensions that conflict with each other than we shouldn't give extension A the ability to suppress extension B. Instead, all features should be configurable so that these conflicts can be avoided. What extensions or VS Code can to is provide guidance and explain what's happened, like VS Code could try to explain what's happening before save and why or editor-config could know about some conflicting defaults in vscode, like files.insertFinalNewLine (which at least can be configured per file type)

@nrayburn-tech
Copy link
Contributor

editor-config could know about some conflicting defaults in vscode

This is simple enough for the current save participants.

VS Code could try to explain what's happening before save and why

This is probably more complex than checking for conflicts and I am not sure that this is needed in the case of editorconfig.

With the current save participants that VS Code has, there probably aren't any changes that need to be made in VS Code. Extensions can update and notify the user of a conflict through notifications, status bar, logs, etc. These notifications can guide the user to configuring their workspace settings to avoid the conflicting configurations. Is this right?

@SamB
Copy link
Contributor

SamB commented Jun 8, 2021

Well, I don't think anybody is really asking VS Code to worry about what happens when eslint config and .editorconfig disagree; just that there needs to be a way that either extension can override the default or user-level VS Code settings. (Overriding workspace-specific settings wouldn't really hurt either, but it's less essential as the user could reasonably be instructed "don't do that, then" w.r.t conflicting workspace settings, just as for conflicts between editorconfig and jslint.)

@Anutrix
Copy link

Anutrix commented Feb 15, 2024

The EditorConfig bug almost made me permanently get rid of VSCode because there was no way to find which extension was doing 'Remove space on save'.

Wish there was some logs to find out which extension is messing up when I do something basic like save a file.

@sanmai-NL
Copy link

You can save without formatting too. The meaning of saving is clearly documented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api feature-request Request for new features or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants