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

feat: add support for files.insertFinalNewline during formatOnSave #13751

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

xai
Copy link
Contributor

@xai xai commented Jun 3, 2024

What it does

When the files.insertFinalNewline property is set to true, formatOnSave() will ensure that the file ends with a newline character instead of always removing a final empty line.

Fixes #13475

Contributed on behalf of STMicroelectronics

How to test

  1. Open a workspace and set "editor.formatOnSave" to true in the workspace settings.
  2. Edit a file that has a final empty line and save.
  3. Verify that the empty line has been removed.
  4. Set "files.insertFinalNewline" to true in the workspace settings.
  5. Verify that final empty lines are kept or added when saving.

Follow-ups

Review checklist

Reminder for reviewers

@JonasHelming
Copy link
Contributor

@xai What is the state of this?

@xai
Copy link
Contributor Author

xai commented Jun 26, 2024

@xai What is the state of this?

As I commented in the related issue, I feel that the added protected function might be living in the wrong place. But it works, and I do not know what would be a better file/package for that function, so I am just going to remove the draft state now and hope for input from a reviewer.

@xai xai marked this pull request as ready for review June 26, 2024 09:05
@JonasHelming JonasHelming requested a review from tsmaeder July 1, 2024 07:19
@xai
Copy link
Contributor Author

xai commented Jul 1, 2024

@tsmaeder alerted me that the property seems to be ignored in certain cases:

  • It works as intended without the built-in extensions (i.e., omit yarn download:plugins while building or clear plugins/ directory).
  • With all built-in extensions, the behavior of an extension seems to override the new files.insertFinalNewline setting and also the format-on-save setting.

Merging as is does not make sense, because it will not work in most cases (i.e., all built-in extensions are present). So, I am going to debug into this to find out which extension we are having this feature interaction with.

@xai
Copy link
Contributor Author

xai commented Oct 29, 2024

FYI: I am retesting this right now ...

Edit:
One of the plugins that is definitely interfering with the files.insertFinalNewline setting in Theia, is editorconfig.editorconfig, which is also part of the default built-ins.

If your workspace contains a .editorconfig file and specifies insert_final_newline, the newline will be added if theiasetting || editorconfigsetting is true.

@xai
Copy link
Contributor Author

xai commented Oct 29, 2024

Just rebased against master, above comment still applies.

When the `files.insertFinalNewline` property is set to true,
formatOnSave() will ensure that the file ends with a newline character
instead of always removing a final empty line.

Contributed on behalf of STMicroelectronics

Signed-off-by: Olaf Lessenich <[email protected]>
@xai
Copy link
Contributor Author

xai commented Oct 29, 2024

So, what is the expected behavior if a generic Theia workspace setting such as files.insertFinalNewline and extension settings (e.g., in editorconfig, or any language-specific vscode extension) mismatch? From a user perspective, I would probably assume that the more specific setting (i.e., typically the extension) takes precedence. Any opinion, @tsmaeder?

@tsmaeder
Copy link
Contributor

@xai We can turn the setting on/off per language in the settings, so I think doing it after the "format on save" is the right thing to do.

@tsmaeder tsmaeder merged commit ecedc9b into eclipse-theia:master Nov 28, 2024
11 checks passed
@github-actions github-actions bot added this to the 1.56.0 milestone Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Formatter removes final newline on package.json
3 participants