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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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
@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 <olessenich@eclipsesource.com>
@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Waiting on reviewers
Development

Successfully merging this pull request may close these issues.

Formatter removes final newline on package.json
2 participants