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

Mega refactor #22

Merged
merged 12 commits into from Jan 4, 2016

Conversation

Projects
None yet
8 participants
@Mpdreamz
Member

Mpdreamz commented May 25, 2015

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 editorconfig/editorconfig#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:

  • Moved project to VS 2013
  • Moved to .NET 4.5 and using the VS 2013 SDK
  • The above means we will drop support for VS2010 !
  • Fixed project id so that when we publish to the gallery we won't end up with two editorconfigs :)
  • Made sure the new completions and syntax highlighting still work in the refactored project.
  • Moved to managed .net editorconfig core which passes the original c core's tests and several additional C# ones. This makes building the project that much more easier.
  • Finally added an EditorConfig file template, works on Add New Item on solution root or solution folders and will create an .editorconfig file next to the solution file.

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

jedmao and others added some commits Apr 20, 2013

Merge and fix commit, merged @jedmoa's work into
this branch so we can jump on the codebase leveriging ported
infrastructure code from codemaid (with permission). This should yield
better support for restoring global settings.

Merge remote-tracking branch 'jed/enforce' into feature/managed-core

Conflicts:
	Core
	Plugin/Plugin.cs
	Plugin/Plugin.csproj
	Plugin/PluginFactory.cs
	Plugin/source.extension.vsixmanifest
	readme.md
	release-notes.md
@Mpdreamz

This comment has been minimized.

Member

Mpdreamz commented May 26, 2015

  • This branch will try to clean readonly files.
  • Clean on open should be turned off by default IMO.
@@ -5,3 +5,5 @@ Martijn Laarman
Hong Xu
Tom Potts
Ethan J. Brown
Jed Hunsaker

This comment has been minimized.

@jedmao

jedmao May 26, 2015

Member

My name has changed, legally, to Jed Mao since then.

@jedmao

This comment has been minimized.

Member

jedmao commented May 26, 2015

👍 I agree that clean on open should be removed.

removed cleanup on open (much snappier) and always output file settin…
…gs into the editorconfig output pane upon opening a file[
@Mpdreamz

This comment has been minimized.

Member

Mpdreamz commented May 27, 2015

  • Clean up on open file is gone
  • Opened file's settings are always outputted in the EditorConfig output pane.

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

  • Refactorings from e.g resharper do not take editorconfig settings into account.
  • Changing indent_size while using tabs can have a very jumpy effect when the current editor receives and loses focus (e.g when jumping to solution explorer).

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

@jedmao

This comment has been minimized.

Member

jedmao commented May 27, 2015

@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

This comment has been minimized.

Member

Mpdreamz commented May 27, 2015

thanks for posting the original reason @jedmao 👍

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 😄

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

@jedmao

This comment has been minimized.

Member

jedmao commented May 27, 2015

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

@pruiz

This comment has been minimized.

pruiz commented Jul 2, 2015

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

@Mpdreamz

This comment has been minimized.

Member

Mpdreamz commented Jul 2, 2015

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

This comment has been minimized.

ctlajoie commented Aug 7, 2015

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

This comment has been minimized.

Member

Mpdreamz commented Sep 17, 2015

@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

This comment has been minimized.

ctlajoie commented Sep 18, 2015

@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.

@jedmao

This comment has been minimized.

Member

jedmao commented Sep 18, 2015

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

@Manuzor

This comment has been minimized.

Manuzor commented Dec 11, 2015

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

@drauch

This comment has been minimized.

drauch commented Jan 4, 2016

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

This comment has been minimized.

Member

Mpdreamz commented Jan 4, 2016

@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.

Mpdreamz added a commit that referenced this pull request Jan 4, 2016

@Mpdreamz Mpdreamz merged commit dae85de into master Jan 4, 2016

@ygoe

This comment has been minimized.

ygoe commented Jan 13, 2016

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

This comment has been minimized.

drauch commented Jan 13, 2016

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

@ygoe

This comment has been minimized.

ygoe commented Jun 17, 2016

@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.

@jedmao jedmao deleted the feature/managed-core branch Jun 17, 2016

@Mpdreamz Mpdreamz referenced this pull request Jun 20, 2016

Closed

[VS] Add new template #175

@sandcastle

This comment has been minimized.

sandcastle commented Sep 28, 2016

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

@ygoe

This comment has been minimized.

ygoe commented Sep 28, 2016

@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

This comment has been minimized.

sandcastle commented Sep 28, 2016

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

@ygoe

This comment has been minimized.

ygoe commented Sep 28, 2016

@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

This comment has been minimized.

Member

Mpdreamz commented Sep 28, 2016

@ygoe "this abandoned broken extension" 👎 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! 😄

@ygoe

This comment has been minimized.

ygoe commented Sep 28, 2016

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment