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

editorconfig: Apply properties to Monaco editor when opening/switching editor #4584

Merged
merged 1 commit into from
Mar 22, 2019

Conversation

simark
Copy link
Contributor

@simark simark commented Mar 15, 2019

Since commit 260d794 ("[editorconfig] Don't apply all properties on
open"), the editorconfig settings are not applied to the Monaco editor.
For example, if my .editorconfig file says that *.py files should use
tabs with a tab width of 7, I would expect the Monaco editor to be set
to "tab" and a tab width of 7 when I open a .py file. At the moment,
this does not happen.

The method that applies these properties to the Monaco editor is
applyProperties. This method is however not called anywhere outside
tests (the commit mentioned above removed the only call).

I have added back the call, such that when an editor is created or the
current editor changes, we apply the settings from the .editorconfig to
the Monaco editor.

I verified that the behavior the commit mentioned above meant to fix is
kept. That is, no modification is done to the file contents as a result
of just opening the file (or switching to and editor where it is open).
Even if I have "trim_trailing_whitespace = true" in my .editorconfig,
opening the file does not cause the trailing whitespaces to be trimmed.
Pressing ctrl-s, however, does (even if the editor doesn't appear as
"dirty").

Fixes #4308

Signed-off-by: Simon Marchi simon.marchi@efficios.com

…g editor

Since commit 260d794 ("[editorconfig] Don't apply all properties on
open"), the editorconfig settings are not applied to the Monaco editor.
For example, if my .editorconfig file says that *.py files should use
tabs with a tab width of 7, I would expect the Monaco editor to be set
to "tab" and a tab width of 7 when I open a .py file.  At the moment,
this does not happen.

The method that applies these properties to the Monaco editor is
applyProperties.  This method is however not called anywhere outside
tests (the commit mentioned above removed the only call).

I have added back the call, such that when an editor is created or the
current editor changes, we apply the settings from the .editorconfig to
the Monaco editor.

I verified that the behavior the commit mentioned above meant to fix is
kept.  That is, no modification is done to the file contents as a result
of just opening the file (or switching to and editor where it is open).
Even if I have "trim_trailing_whitespace = true" in my .editorconfig,
opening the file does not cause the trailing whitespaces to be trimmed.
Pressing ctrl-s, however, does (even if the editor doesn't appear as
"dirty").

Fixes #4308

Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
@simark
Copy link
Contributor Author

simark commented Mar 15, 2019

@adrienrenaud, could you give this a try?

Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works fine here, awesome!

@@ -98,6 +98,7 @@ export class EditorconfigDocumentManager {
const uri = editor.uri.toString();
this.editorconfigServer.getConfig(uri).then(properties => {
this.properties[uri] = properties;
this.applyProperties(properties, editor);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@svenefftinge Did you remove it on purpose?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change is ok, otherwise settings like end_of_line won't work as expected.

Copy link
Member

@akosyakov akosyakov Mar 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@kittaakos kittaakos removed their request for review March 18, 2019 08:17
@simark
Copy link
Contributor Author

simark commented Mar 22, 2019

Ping.

@AlexTugarev
Copy link
Contributor

I'm testing it right now.

@simark
Copy link
Contributor Author

simark commented Mar 22, 2019

I'm testing it right now.

Thanks!

@AlexTugarev
Copy link
Contributor

2019-03-22 16 27 40

Unfortunately, the effect of this change is super weird. I guess it's colliding with the settings when trying with e.g. intend_size. In the debugger I could see, this change was applied to the monaco model, but the change was not visible. On save it was then updated. (Undo appended a new empty line at the end, which might be just a follow up error.)

@simark
Copy link
Contributor Author

simark commented Mar 22, 2019

Sorry, I don't understand the problem you are trying to describe. Could you describe the steps you are doing in this video clip?

@AlexTugarev
Copy link
Contributor

The confit is not applied when opening the editor. In that case I‘ve added indent_size for json finds, but there seems to be no effect on open, only on save.

@simark
Copy link
Contributor Author

simark commented Mar 22, 2019

The confit is not applied when opening the editor. In that case I‘ve added indent_size for json finds, but there seems to be no effect on open, only on save.

Can you define what you mean by "the config is not applied".

What I expect my editor to do is just set the whitespace settings to what's defined in the .editorconfig file, such that when I'll press tab, or ask it to auto-format, it will use these settings. But I don't expect the editor to modify the content of the file to match what's in the .editorconfig file just when I open a file.

So let's say your .editorconfig says to use indentation of 4 spaces for json files. You open a json file that is currently indented with 2-spaces indents. Opening the file should not change the file content to now have 4 spaces. However, the whitespace settings (what you see in the status bar) should say "4 spaces". If you ask Theia to auto-format the file (or save while having "format on save enabled"), it should do so using 4 spaces.

The behavior with this patch applied matches what I expect. If you think the behavior is not the one we should have, please state clearly which steps you take, which behavior you see, and which behavior you would expect to see.

@AlexTugarev
Copy link
Contributor

You are right! It works as expected. The insertion of a new line on undo is a separate issue.

Copy link
Contributor

@AlexTugarev AlexTugarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works nicely! Thanks!

@simark
Copy link
Contributor Author

simark commented Mar 22, 2019

Ah ok I see what you mean by "undo" inserted a new line. It doesn't look related to this patch.

@simark
Copy link
Contributor Author

simark commented Mar 22, 2019

Thanks for the reviews, I am merging it.

@simark simark merged commit 7d13d59 into master Mar 22, 2019
@simark simark deleted the simark/editorconfig-fix branch March 22, 2019 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

editorconfig not working as expected
4 participants