search

editorconfig / editorconfig-visualstudio

EditorConfig Visual Studio Addin
http://editorconfig.org
Other
352 stars 74 forks source link

Mega refactor #22

Closed Mpdreamz closed 8 years ago

Mpdreamz commented 9 years ago

This PR represents @jedmao hard work refactoring the internals of EditorConfig VS extension. It copies parts of the codemaid architecture (with permission) in order to create a smoother experience hooking into the needed events in the editing pipeline.

This was never properly released. Only builds distributed on issues, and once, shortly, using the wrong project id on the gallery (causing two editorconfig extensions to be available). See for more information https://github.com/editorconfig/editorconfig/issues/173

This should fix not restoring global settings properly on for projects/solutions that do not have an editorconfig file.

Other changes bundled into this PR:

image

It relies on a hack, showing an ugly filename to the user, to create a file with only an extension (which is not allowed in the dialog) image

but will always ignore whatever is passed and always create a file called .editorconfig

image

I really hope to make time to set up a test box for VS2012 and VS2013, so far I have tested this against VS 2015 Community. This PR might linger a while since I'll be on a 3 week holiday a week from now. If you have VS2012 and VS2013 please help by trying this

http://editorconfig.blob.core.windows.net/vsix/EditorConfig.vsix

and reporting results back on this ticket

ping @jedmao @nulltoken @swansontec @KindDragon

Mpdreamz commented 9 years ago
jednano commented 9 years ago

:+1: I agree that clean on open should be removed.

Mpdreamz commented 9 years ago

The restore of global settings is too aggressive on this branch:

Going to experiment using restoring settings on closing the editor as suppose to on lost focus

jednano commented 9 years ago

@Mpdreamz the reason global settings are restored when a tab loses focus is so they can edit their global settings w/o being confused about what settings they are editing.

Mpdreamz commented 9 years ago

thanks for posting the original reason @jedmao :+1:

This conflicts with losing focus because of a refactor dialog popping up (either resharper or visual studio's own). Kind of deadlocked here haha :)

In my opinion having refactorings respect editor settings is more important then a confusing text editor settings dialog in visual studio (which is already pretty confusing if you ask me). Also the whole point of .editorconfig is to no longer rely on that dialog in the first place :smile:

The prior you will hit more often then the latter and we can only fix one AFAIK.

jednano commented 9 years ago

As long as you still restore the EditorConfig settings on tab focus, I support your strategy.

pruiz commented 9 years ago

Are there any pending issues impeding merge of this PR? :?

Mpdreamz commented 9 years ago

yes refactoring (roslyn based or resharper based) does not take editorconfig settings in to account because we reset very aggressively.

I've been running every day with the extension (0.6 as linked in this thread) now and VS2015 often crashes, this could be because of other extensions or vs 2015 RC itself. I'm now (as of today) running on the published version of editorconfig again to compare how often VS 2015 crashes.

Definitely not ready to pull the trigger and publish this yet though.

ctlajoie commented 8 years ago

Is this still progressing? I found a bug related to file paths that I'd like to submit a PR for, but it looks like it would conflict with this one if I did... Disregard the bug I mentioned. The changes in this PR fix the bug.

Mpdreamz commented 8 years ago

@ctlajoie I want to get back to this but I have and will be too swamped to spend brain cycles on this.

The main gripe I have with this branch that needs fixing is that it restores settings to aggressively so that when you do new file or any refactoring it does not take into account the editorconfig settings until you an explicit code-format.

ctlajoie commented 8 years ago

@Mpdreamz Can I humbly suggest that this PR be merged so anyone else (if so inclined) can fix the problems? I have noticed a few issues with this version myself (none severe though), but have avoided looking for the problem until this gets merged so I can potentially submit my own PR.

jednano commented 8 years ago

Aren't these "issues" pre-existing anyway? Then what are we losing by merging this PR in?

Manuzor commented 8 years ago

So, what's the status of this? It sounds very exciting and I'm looking forward to an improved VS2015 experience with EditorConfig!

drauch commented 8 years ago

We are currently evaluating whether we want to use editorconfig with Visual Studio, however, we are not sure whether the project is still maintained. Is this branch being merged anytime soon or is editorconfig-visualstudio abandoned?

Sorry for the harsh words, I'm sure you have a ton of stuff to do and this is only a spare-time project for the community, still, if you don't have time, please say so, maybe someone else wants to fill in or at least we know, that it doesn't make sense to wait for this PR to be merged.

Mpdreamz commented 8 years ago

@drauch I am itching to jump back on it. I have no intend to give up on this branch just been swamped with deliverables at work for the past months. I have moved back to the gallert published version of editorconfig (in vs2015) because I got bit by the refactorings not taking settings into account bug on this branch and that was impeding my work.

I am going to merge this in as is, so we can move forward on this.

ygoe commented 8 years ago

Is this a fix to #18? If so, that's definitely interesting as that bug is a blocker for me.

Do I understand it right that settings are currently restored whenever the editor loses focus? And that it was discussed to change that to when the editor is closed instead? Sounds a lot more reasonable to me. What about multiple editors open in parallel? Can they conflict? Say if two files from different projects with different editorconfig settings are open.

How does the restoring work? Would the original user preferences be lost forever when Visual Studio crashes while a file with different settings is open?

drauch commented 8 years ago

@dg9ngf: :+1: would like to know answers to all your questions as well!

ygoe commented 8 years ago

@Mpdreamz Any news on this? I see project activity, but this blocking bug is not progressing anyhow. What's your roadmap or definition of priorities? Features before bugs? I'd love to use this, but I can't if it messes up my IDE.

sandcastle commented 7 years ago

@Mpdreamz How can we help to move this forward? It has been months with no love 😿 💔

ygoe commented 7 years ago

@sandcastle Just wait for Visual Studio "15", it will have EditorConfig support included. No need for this abandoned broken extension anymore! :-) See https://visualstudio.uservoice.com/forums/121579-visual-studio-2015/suggestions/6146845-support-editorconfig

sandcastle commented 7 years ago

@ygoe that's great news, do you know if it's already in the pre-RTM version atm?

ygoe commented 7 years ago

@sandcastle There's a link to a video in their announcement. I've watched that and it looks like this feature is going to be in the next preview (> 4) or whatever comes next. I haven't heard of an estimated RTM release date yet but expect it to be sometime in Q1/2017.

Mpdreamz commented 7 years ago

@ygoe "this abandoned broken extension" :-1: wording matters this is a labor of love project in our spare time.

Both published and refactored versions have different interoperabillity problems that are hard to tackle. The published version just has the least intrusive problems and works well for many people.

VS15 getting editorconfig support takes some of the pressure of and will be much welcomed! It will extend in to roslyn formatting settings too so good times ahead! :smile:

ygoe commented 7 years ago

@Mpdreamz I'm sorry that my description sounds bad to you, nonetheless I consider it valid (at its time) based on public observations: this extension has (or had? we don't know) a major bug that I personally could not accept and nobody cared to respond. Communication has a big effect on how something is perceived, and the lack of communication doesn't improve my trust.