Can use incorrect space/tab setting when switching between split screen windows

[murrayju]

I suspect that this is caused by an interaction between this extension, and two others that I am using:

• Autosave on View Change
• EditorConfig

The case is, I have two files open in split screen mode. One file is using spaces, the other is using tabs (this happens automatically based on my EditorConfig, one is HTML, the other is JS). If I make some edits in the first file, then switch over to the second file, the autosave plugin saves the first file, which in turn runs this wsSanitizer plugin. The problem is that it uses the spaces/tabs preference from the second file instead of the first.

[kidwm]

Hi, I'm the maintainer of brackets-editorconfig, I don't recommend you use wsSanitizer and EditorConfig together, since these two share a lot of same codes and do the similar job.

[MiguelCastillo]

@murrayju I am sorry I didn't notice your issues! I haven't used EditorConfig.

@kidwm Pity there was never a pull request... It confuses users when extensions overlap and conflict. Do you think you can productize reditorconfig? Maybe we can make a standalone module that other extensions can consume, including wsSanitizer.

[kidwm]

@MiguelCastillo That would be great, but I have no idea how to do that. Maybe I should remove the same sanitize part with wsSanitizer in brackets-editorconfig to avoid the conflict and push user to install these two extensions both?

[MiguelCastillo]

@kidwm The idea is to make editorconfig a generic extension that can broadcast events about changes to the config file, where extensions can opt in to participate in getting config changes. It seems like you are already writing settings to PreferencesManager so extensions can just use that as the event bus. And you are also loading the config files based on the current document. What happens if the config file isn't found where the current document is?

Let me take a close look at what's happening in editorconfig so that we can brainstorm and make this happen. It seems like you just need to blinding write all settings found in the config file into the PreferencesManager. And all an extension would need to do is listen to PreferencesManager changes.

[MiguelCastillo]

@kidwm How feasible is it for you to write the entire config file you to the PreferencesManager? https://github.com/kidwm/brackets-editorconfig/blob/master/main.js#L152

Doing that will allow extensions to register for preference changes to this https://github.com/kidwm/brackets-editorconfig/blob/master/main.js#L23

[kidwm]

@MiguelCastillo

What happens if the config file isn't found where the current document is?

As you see, the EditorConfig JavaScript core will return an empty object and nothing happens https://github.com/kidwm/brackets-editorconfig/blob/master/main.js#L152

BTW, I didn't make a pull request for EditorConfig that is just because of this EditorConfig JavaScript core library, I don't think it should be part of wsSanitizer.

[MiguelCastillo]

@kidwm Yeah I agree editorconfig does not belong in wsSanitizer and understand why it wasn't a PR. I think editorconfig can be a really great standalone module used by wsSanitizer, which would eliminate having two wsSanitizers. If we can make that work, than I will be integrating all my other extensions as well! :)

What are your thoughts on my comments about having editorconfig write everything it finds into PreferencesManager and letting extensions register for change events?

[kidwm]

@MiguelCastillo Actually, I forgot that I have commented out the sanitize part codes in bracket-editorconfig, so it has nothing to do with this issue.

And for you question, I've tried to solve this issue, but found there is a big problem that Brackets seems not offer a method to store such preferences for each file, so it is hard for wsSanitizer to get the right setting after view changed.

Maybe I should write write everything into view state of files?

[murrayju]

Glad to see you two talking, it does seem like the two plugins could be merged.

As far as the bug reported in this issue goes, I think it has more to do with how wsSanitizer is grabbing the whitespace settings of the incorrect file (due to a race condition where the file is saved after switching to a different tab). I only mentioned EditorConfig as being involved because it is setting the whitespace settings for each file.

[MiguelCastillo]

@murrayju I think I see what's happening... This issue seems to be right here https://github.com/MiguelCastillo/Brackets-wsSanitizer/blob/master/main.js#L50

I am grabbing the setting from Editor, which perhaps is the new document... I need to debug this one a bit.

[kidwm]

@MiguelCastillo This my attempt to get the correct setting from PreferencesManager with filePath argument assigned instead of Editor. https://github.com/kidwm/brackets-editorconfig/commit/e97f4fee491e4d79e8fa427072f4188d1d3a1538

But it was not working since the id of preference item is unique regardless of filePath as context.

[MiguelCastillo]

That looks really promising. Let me give it a quick test.

[MiguelCastillo]

@kidwm I came up with something a bit different. The issue to me is that we want to sanitize a document upon saving it, and the trick was to figure out how to get the correct settings. I am hoping this works :D My tests showed that it did.

@murrayju Do you think can try latest from master branch to see if this resolves the issue for you, please?

[kidwm]

@MiguelCastillo It would not work, I've tried to set context in brackets-editorconfig to test this method but in vain.

I'll try to set PathLayer in Preference to see if it solve this issue.

[MiguelCastillo]

@kidwm What wouldn't work? Getting the setting with fullpath?

[kidwm]

@MiguelCastillo yes, but I didn't know if I did setting correctly.

[MiguelCastillo]

@kidwm Getting the settings with the full path for the document should work. That's how we do it in the editor/Editor.js. I would be very surprised if it didn't and would probably log a bug.

[kidwm]

@MiguelCastillo This is my test case, did I do something wrong?

var MainViewManager    = brackets.getModule('view/MainViewManager');
var PreferencesManager = brackets.getModule('preferences/PreferencesManager');
var files = MainViewManager.getAllOpenFiles();
PreferencesManager.set("useTabChar", true, {context: files[0].fullPath});
PreferencesManager.get("useTabChar", files[0].fullPath);
PreferencesManager.set("useTabChar", false, {context: files[1].fullPath});
PreferencesManager.get("useTabChar", files[1].fullPath);
PreferencesManager.get("useTabChar", files[0].fullPath);

[kidwm]

@peterflynn Can verify this test case in my previous comment?