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

editor.formatOnSaveMode does not appear to function #105800

Closed
alexdima opened this issue Sep 1, 2020 · 11 comments
Closed

editor.formatOnSaveMode does not appear to function #105800

alexdima opened this issue Sep 1, 2020 · 11 comments
Assignees
Labels
formatting Source formatter issues info-needed Issue requires more information from poster
Milestone

Comments

@alexdima
Copy link
Member

alexdima commented Sep 1, 2020

Testing #105733

  • Create a file file.ts
  • Paste the following contents:
class A {
private foo(): number {
return 5;
}
}
  • Close the file and reopen vscode
  • Configure "editor.formatOnSaveMode": "modifications"
  • Configure "editor.formatOnSave": true
  • Paste the following contents in the file:
class B {
private foo(): number {
return 5;
}
}
  • Save file via cmd+s to get the format on save running
  • Observe that class A has been formatted:

Kapture 2020-09-01 at 10 52 28

@jrieken
Copy link
Member

jrieken commented Sep 1, 2020

@alexdima modifications are based on SCM diff information. Your file appears as untracked and no quick diff markers show. Do you really have only those lines "SCM modified"?

@jrieken jrieken added the info-needed Issue requires more information from poster label Sep 1, 2020
@miguelsolorio
Copy link
Contributor

I can also repo, I have the same settings from above ("editor.formatOnSaveMode": "modifications", and "editor.formatOnSave": true) and I expect only to have line 10 formatted but then other lines get formatted.

Kapture 2020-09-01 at 14 45 01

@jrieken
Copy link
Member

jrieken commented Sep 2, 2020

@misolori you are on the right path. Modifications are being formatted but my assumption is that I pass the range 10,1-11,1 to the html formatter and that it then over-eagerly formats the whole statement of line 11.

@alexdima ping for clarification

@jrieken jrieken added this to the August 2020 milestone Sep 2, 2020
@jrieken jrieken added the formatting Source formatter issues label Sep 2, 2020
@alexdima
Copy link
Member Author

alexdima commented Sep 2, 2020

@jrieken I see, I misunderstood the feature. Maybe the setting could make it more clear that this is only about SCM diff ranges. I thought that the file is read from disk just before saving and a diff is computed with the state on disk. It was not clear to me that the diff is computed with the state of the index.

@jrieken
Copy link
Member

jrieken commented Sep 2, 2020

The setting says "requires source control"... I actually like your assumption of how it should/could work and that's maybe something for the future. So, maybe we find an alternative for "modified" that expresses it's "source control modified" and then, in the future, we have another option that says "modified in session". Ideas?

Screenshot 2020-09-02 at 09 58 47

@alexdima
Copy link
Member Author

alexdima commented Sep 2, 2020

👍 Sorry about my confusion. I didn't use the settings ui and this was my experience:

image

@jrieken
Copy link
Member

jrieken commented Sep 2, 2020

Yeah, the suggestion details widget would show this... Still open for suggestion for a better name and maybe give this a short test run after all

@misolori can you share the file and what formatter you are using? I wasn't able to reproduce what you see.

@miguelsolorio
Copy link
Contributor

So it looks like that may have been an issue with my formatter (Prettier). Disabled that and used the default one and got it working for JS files.

However, I'm not seeing this work reliably with JSON files. Some format commands don't work (Format Selection and Format Modified Lines) but Format Document does work 🤷‍♂️:

Kapture 2020-09-02 at 08 13 16

@jrieken
Copy link
Member

jrieken commented Sep 2, 2020

Yeah, the secret of "format modified" is that it does "format selection" where the selection isn't visible and consists of the modified ranges. Adding @aeschli who did the JSON formatter - it might not be fit for arbitrary range formattings

@miguelsolorio
Copy link
Contributor

Seeing similar things with the HTML formatter. This works fine in other JS/TS files though. Hopefully these bugs can get fixed and are not by design.

@jrieken
Copy link
Member

jrieken commented Sep 3, 2020

I have created #105998 to track selection formatting issues. Closing this as the @alexdima's confusion has been cleared

@jrieken jrieken closed this as completed Sep 3, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Oct 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
formatting Source formatter issues info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

3 participants