search

Tom-Bonnike / vscode-formatting-toggle

A VS Code extension that allows you to toggle the formatter (Prettier, Beautify, …) ON and OFF with a simple click.
https://marketplace.visualstudio.com/items?itemName=tombonnike.vscode-status-bar-format-toggle
MIT License
60 stars 13 forks source link

Implement disabling formatting on save via keybinding override #64

Closed zardoy closed 2 years ago

zardoy commented 2 years ago

There are a lot of VSCode settings that makes IDE format the code on save:

Also extensions can format documents via vscode.workspace.onWillSaveTextDocument. I don't think it can be disabled with any setting.

It is much easier and more reliable to disable any possible formatting via overriding ctrl+s shortcut to workbench.action.files.saveWithoutFormatting.

I propose to add a boolean setting which would also enable this behavior.

I'd like to implement this, however I still in doubt how we can write to keybindings.json. AFAIK we don't have direct access to neither settings.json nor keybindings.json via API, but we want this to also work in web.

zardoy commented 2 years ago

Or maybe we should introduce a setting like editor.disableFormatting in VSCode?

Tom-Bonnike commented 2 years ago

👋 Interesting. First thoughts:

  1. I propose to add a boolean setting which would also enable this behavior.

    Indeed, it should be an option. There are probably people that want to leave some of the formatting settings enabled. Most people probably really only care about disabling formatOnSave, formatOnPaste and formatOnType. I know that’s my case and that’s why I created this extension in the first place.

  2. “Save without formatting” might not prevent the editor.codeActionsOnSave or vscode.workspace.onWillSaveTextDocument updates you mentioned.

  3. If no one complained except about https://github.com/Tom-Bonnike/vscode-formatting-toggle/issues/54 (which only 3 people upvoted), it’s probably not actually a change that many people actually need?

  4. Or maybe we should introduce a setting like editor.disableFormatting in VSCode?

    That would make it reaaally simple to implement using the existing formattingToggle.affects setup and would make sure we are covering all formatters at once.

I'd like to implement this, however I still in doubt how we can write to keybindings.json. AFAIK we don't have direct access to neither settings.json nor keybindings.json via API, but we want this to also work in web.

I’d happily support you from the sidelines but if I’m being honest I don’t really want to spend time looking into that myself right now.

zardoy commented 2 years ago

2.

“Save without formatting” might not prevent ...

Are you sure? From my experience it always just saves the file without any modifications.

Just look here:

As you can see forementioned settings, code actions and vscode.workspace.onWillSaveTextDocument are skipped with that command.

3.

probably not actually a change that many people actually need

I just expect from extension that it works in 100% cases. You know, it might be annoying to have a lot of pitfals and I don't want to learn where it works and where it doesn't.

1.

... I know that’s my case and that’s why I created this extension in the first place.

In my case I just want to avoid any unexpected diffs in forks. For example I'm globally using Prettier and forementioned on-save formatting settings. Sometimes formatter isn't configured properly, sometimes files.trimTrailingWhitespace just removes all whitespaces from the file, which in turn creates huge diffs on GitHub.

Here is my final proposal:

Since it's not possible to modify user shortcuts, introduce a formattingToggle.toggleFormat command and formattingToggle.disableFormatting setting. Suggest user to bind this command to his save shortcut (ctrl+s). Whenever formattingToggle.disableFormatting is enabled (either in workspace level (https://github.com/Tom-Bonnike/vscode-formatting-toggle/issues/61) or globally) it would call workbench.action.files.saveWithoutFormatting or default workbench.action.files.save otherwise.

As you can see, it would resolve all issues in this repo and more importantly my needs.

Of course I could implement this in a new extension as it is absolutely different behavior, however maybe you don't mind of introducing formattingToggle.toggleMode setting:

Then we can introduce a command to toggle the setting on workspace-level.

I’d happily support you from the sidelines but if I’m being honest I don’t really want to spend time looking into that myself right now.

No worries I can do anything myself.

Tom-Bonnike commented 2 years ago

As you can see forementioned settings, code actions and vscode.workspace.onWillSaveTextDocument are skipped with that command.

Ah, looks like it indeed. Thanks for looking.

Since it's not possible to modify user shortcuts

Aw, that would have been a much nicer solution. Maybe there’s a way to hook into a save event and somehow prevent the default behaviour? Let’s just make really sure there isn’t a more elegant solution before forcing people to rebind their shortcuts. Might be worth asking the folks over at VSCode if they have an opinion? The extension has quite a lot of installs so I’m sure we could get some eyes from them.

introduce a formattingToggle.toggleFormat command and formattingToggle.disableFormatting setting. Suggest user to bind this command to his save shortcut (ctrl+s). Whenever formattingToggle.disableFormatting is enabled (either in workspace level (#61) or globally) it would call workbench.action.files.saveWithoutFormatting or default workbench.action.files.save otherwise.

  • when setting: use existing behavior
  • when command: just toggle formattingToggle.disableFormatting setting

Sounds like a decent workaround yeah! Let’s make sure we get the naming right, the extension will be offering two ways, we should make the pros and cons obvious in the README (I can help with that once the PR ships).

I think we don’t need formattingToggle.toggleMode. What do you think of:

  // Super clear name, this way if someone is just going through the VSCode settings
  // they’ll know they need to do something else for it to work.
  // Probably we can add some description to the setting with instructions.
  "formattingToggle.forceFormattingStatusViaKeybindOverride": true,

  // Would have no effect if the above is set, we could make that clear in the README.
  "formattingToggle.affects": [
    "editor.formatOnPaste",
    "editor.formatOnSave",
    "editor.formatOnType"
  ]
zardoy commented 2 years ago

Sorry for that delay. I just found that it's possible to achieve this without any extension.

Just add this to keybindings.json

{
    "key": "cmd+s", //ctrl+s for win / linux
    "command": "workbench.action.files.saveWithoutFormatting",
    "when": "config.saveWithoutFormatting"
}

And just toggle (true/false) saveWithoutFormatting setting globally / per project.

Its possible to toggle that setting with toggle extension via keybinding, or with commands to toggle via statusbar.

As an alternative solution for this issue, we probably can:

{
    "key": "ctrl+s",
    "command": "workbench.action.files.saveWithoutFormatting",
    "when": "config.formattingToggle.forceDisableFormattingViaKeybinding"
}

Anyway, we can't just hook into other commands. Of course, you can try to open feature request for that, but I won't even try. I can imagine this is an obvious security issue (for the same reason we can't read arbitrary when context from extension host).

However, solution from previous comments is much better.

Also, I don't understand the purpose of formattingToggle.forceFormattingStatusViaKeybindOverride, I wanted to add formattingToggle.toggleMode to toggle the mode of the extension (which setting will toggle statusbar button).

At the moment this is not in priority list, as I've found workaround that I described above, sorry.

Tom-Bonnike commented 2 years ago

Alright; since you’re the only one that requested something like this, I’ll close for now. Thanks anyway!