alefragnani / vscode-separators

Separators Extension for Visual Studio Code
GNU General Public License v3.0
40 stars 6 forks source link

[BUG] - Formatting places separators on wrong lines #9

Closed JCKodel closed 2 years ago

JCKodel commented 3 years ago

Environment/version

Extension version: v1.0.2 VSCode version: Code 1.51.0 (fcac248b077b55bae4ba5bab613fd6e9156c2f0c, 2020-11-05T18:14:40.758Z) OS version: Darwin x64 19.6.0

Steps to reproduce

  1. Create a Flutter project
  2. Write some code
  3. Save to trigger dart_fmt (the default Dart formatter)

image

The separators are not refreshed when the editor is changed after the format (which is automatic for Dart). Changing something and re-saving triggers the Separators plugin and fixes the issue.

Suggestion: refresh the separators on save and format events.

Detail: my save has a macro to run a plugin to organize imports, then save. Don't know if this is causing the issue or the formatter itself.

alefragnani commented 3 years ago

Hi @JCKodel ,

How do you call/run this dart_fmt Formatter? Is it incorporated in the Dart extension you use (registered as Format Document command) or do you have a Task where you call the formatter externally?

I use two extensions which uses the incorporated model (Format Document command). One was my Pascal Formatter extension, and the other was Prettier, and in both cases, the separators where redrawn correctly. I didn’t test with the external tool approach, so I don’t know how it would work.

Could you provide more details how this dart_fmt works, and how do you run it?

JCKodel commented 3 years ago

The Dart Formatter runs automatically on the save (I'm not exactly sure how, but I definitely don't call anything). The source for Dart VS Code is in https://github.com/Dart-Code/Dart-Code.

I use an extension to fix some glitches on Dart import that I should run manually, but I always forget, so I trigger it on the save option, using this key binding:

{
    "key": "cmd+s",
    "command": "extension.multiCommand.execute",
    "args": {
      "command": "multiCommand.fixAndSave"
    },
    "when": "editorTextFocus && !editorReadonly && resourceExtname == .dart"
  }

This command do the following:

"multiCommand.commands": [
    {
      "command": "multiCommand.fixAndSave",
      "interval": 250,
      "sequence": [
        "dart-import.fix",
        "workbench.action.files.save",
        "notifications.hideToasts"
      ]
    },
    {
      "command": "multiCommand.fixAndSaveAll",
      "interval": 250,
      "sequence": [
        "dart-import.fix",
        "workbench.action.files.saveAll",
        "notifications.hideToasts"
      ]
    }
  ],

That's done because extension writters are amused in triggering notifications for the most useless actions ¬¬

There is no formatter command whatsoever in this pipeline (so I guess is being triggered by Dart itself).

As I'm writing this, I found a new option in the Dart Import Fix that runs automatically on save and testing it made me realize the problem is this extension (Dart Import Fix).

I have deleted all my custom settings and keybindings and after run Fix all imports, this extension opened all my files, did something and at the end it said "No lines changed", but the separator glitched:

image

After holding Ctrl+Z to undo all changed I made in my tests, the separators don't move (I don't think this one behavior is because of the plugin):

image

So, there is something in the redo/undo and in this plugin does that don't trigger the refresh. The source code for this extension is: https://github.com/luanpotter/vscode-dart-import

alefragnani commented 3 years ago

Good to know the issue is not in the Format Document approach. As you could check yourself, it appears the external tool may be the reason.

But I also noticed it uses the Organize Imports command (which if I remember correctly, appeared in VS Code some time ago), and maybe it uses a third way to update the source.

I’ll have to take a look on how these kids works. If they don’t fire the required event onDidChangeTextEditor; if it is fired in the incorrect moment; It fires another event which Separators should look at.

Thank you for the details

jesseeads commented 3 years ago

Windows 10 VSCode ver 1.51.1 Extension 1.0.2 Angular 9 solution

Seems to add extra Separators where they shouldn't be.

Should only be One Separator in the function below but it's placing 3 additional ones.

Great work so far!

image

alefragnani commented 3 years ago

HI @jesseeads ,

Please, take a look on Outline view (in the Explorer Side Bar) and when you call Go to Symbol in the Command Palette. You may notice that VS Code itself recognises these as new symbols.

It appears VS Code identifies callbacks / anonymous methods / arrow functions, as new symbols too, not only those you explicitly declares (like getCustStep).

Hope this helps

alefragnani commented 3 years ago

Hi @jesseeads ,

I was revisiting the opening issues to plan the next release and noticed your comment here. Have you seen that starting in July update (v2.2.0) you can ignore callback/inline (for JS/TS sources)?

If not, please take a look at the separators.functions.ignoreCallbackInline setting (more details here https://github.com/alefragnani/vscode-separators#available-settings).

In the next release (v2.3.0), next week, I'll be supporting Lua language as well. Feel free to open issues for other languages as well.

Hope this helps

alefragnani commented 3 years ago

Hi @JCKodel ,

Are you still experiencing this issue?

I did a few researches back then but didn't find anything that could be missing in the extension's side, to detect those changes in the document, which would require the extension to reload the symbols. I did more researches recently and still no success. But that's an unfortunate scenario, because I never experienced this issue so, it's hard to find a way to fix, unfortunately.

Thank you

alefragnani commented 2 years ago

I'm closing this issue because no new comments has been added since my last question. Feel free to reopen/comment if the error still occurs.

Thanks for your understanding