Addressed #59

[astrohart]

Refactored to separate concerns between ExtensionService, the Command classes, the Extension and GalleryEntry POCOs. This issue is still WIP, but the code is a lot cleaner (according to Uncle Bob's Clean Code book).

[madskristensen]

This is a huge PR with all files being touched. I can't easily tell if this will break any compatibility or change behavior

[astrohart]

I know, it's a lot of files. I hear your concern. It's important to be sure that software works prior to doing a release.

I assure you, I know what I am doing.

However, let's not just take my word for it.

My question would be, may I please create a Unit Tests library using NUnit and then create test cases so, if the unit tests pass, there is a reasonable certainty that the code, as refactored, works?

I can add it to this PR.

That might be the best way forward.

Each commit message has notes below it (if you click the ... button) that says exactly what I did for each commit and my reasons why.

[madskristensen]

Sure, a unit test project would be great

[astrohart]

This is a huge PR with all files being touched. I can't easily tell if this will break any compatibility or change behavior

And I'm only just beginning! I've basically just refactored only one component of the software system -- the ExtensionService and all that it touches.

[astrohart]

Sure, a unit test project would be great

Or project(s). I am using an extension (or plugin) for ReSharper itself, called TestCop for ReSharper and it enforces certain architecture smell rules such as creating unit-testing libraries that goes along with each .csproj or .shproj under test.

In production, software systems have well over 300+ projects. I'll try to keep it to a minimum, but since there is a lot of "clamoring" out in the community for this extension to be shipped pre-installed with Visual Studio, a fully formally-correct, production-level Software Engineering: Principles and Practices approach is, I think, the best way to go.

[astrohart]

ReSharper is a great tool, I swear by it. I have almost 300 snippets that I use (type something and press TAB key) as well as file templates, surround templates, and the actions such as adjusting namespaces, sorting members, or extracting interfaces and pulling members up (or pushing them down) are just commands that I can map keyboard keys to.

When I am programming C# with ReSharper, it's almost like I am playing the keyboard as if it were a piano. I was super inspired last night.

Notice the large number of sloc written per minute -- if you go and see how closely spaced in time the commits are. That is my bevy of snippets at work. I have these huge snippets where I do a TAB and then it writes almost a whole file for me. ReSharper then lets you set placeholders where other information (custom information) can go. It saves a lot of time. It's called their LiveTemplate feature.

It's been a great productivity driver for me, this ReSharper tool.

As a holder of a Ph.D., I was granted a free license to ReSharper through their academic program.

[astrohart]

@madskristensen If you want to check compatibility/see that it still works, I am attaching a ZIP file of the code as of this PR.

ExtensionPackTools.zip

A suggestion might be, you can download this and install it in your VS instances/use the Experimental Instances and verify that the Export Extension, Import Extension and Manage Solution Extension commands all work.

They should unless there is something I changed that broke something, but I am reasonably sure it will test out.

I have only VS 2019 on my box, so I can test it with VS 2019, but not the other two versions of the IDE.

While I am doing that, you can play with it on your side using the attached ZIP, if you want.

I am going to keep filing more commits under this branch in the meantime.

If you get a successful test with my ZIP file then can you be somewhat certain I am heading in an okay direction?

[astrohart]

@madskristensen Tonight, I worked on the ExtensionService some more.

This service has two dependencies, which are injected in the constructor, and are both used in the GetInstalledExtensions method.

I created two other services, ExtensionIdentifierService and ExtensionMetadataService, to wrap these dependencies.

The methods that the services expose do the actions that were initially done in the GetInstalledExtensions method by simply calling thee methods on the dependencies directly.

This is not advisable in production code.

This is because we have no way of knowing whether the third-party SVsExtensionManagerService and SVsExtensionRepositoryService are going to play nice, out of the box, or not.

The ExtensionIdentifierService.GetInstalledExtensionIdentifiers method obtains the list of identifiers for installed extensions that were obtained from the Visual Studio Marketplace, and are not system components, nor parts of packages.

The ExtensionMetadataService.GetExtensionMetadata consumes the list of identifiers and returns an ordered, enumerable collection of instances of Extension (through the IExtension interface) that contain the metadata thusly obtained.

Both methods return the empty collection if invalid (i.e., null inputs are passed, or an exception is thrown during the operation, or if the operation cannot be carried out, for some reason.

This is to make the calls more robust and fault-tolerant.

All the places in constructors where outside dependencies are injected now throw ArgumentNullException if the arguments passed are null.

This is more for the benefit of developers of the system.

This way, we can be sure that if an exception is being raised, it's more of a reminder to us developers to ensure that we are passing in valid object references for the dependencies.

I believe that this is important to do, especially when you are calling on code provided by an external library, or through interprocess communication, which is happening here.

Normally, we can count on such external providers of functionality to work; but due to unforeseen circumstances, such as a network connection not being available, or a server being down, these methods are prone to failure.

We have no idea when or if a failure in the Visual Studio-supplied services may occur.

Therefore, I created these two services to wrap the Visual Studio supplied dependencies and then issue the same calls as previously but this time, in a more fault-tolerant and robust manner.

The methods basically stop and return an empty set in the event of being unable to perform the request operations.

Notice, also, in the ExtensionIdentifierService.GetInstalledExtensionIdentifiers method, which uses a Where filter, I first do a check with Any to see if any results even are returned by our query.

If you chain other methods after Where, such as Select and ToList, sometimes, in specific rare instances, a The sequence contains no elements error may crop up. We short-circuit that by first asking the provider whether, given the same filter expression as in the Where statements, but this time, calling Any, whether any items match the specified criteria. If not, then we give up.

BTW I created a static class IsExtension and a method InstalledByTheUser to fluently wrap the criteria for the ExtensionIdentifierService.GetInstalledExtensionIdentifiers method.

This can then be used as a method group in the Any and Where calls.

The code is much more fluent this way.

Since we are not guaranteed to not be passed nulls by the Visual Studio system, we can also stop the static method inside the method group if we encounter an object or property that is not supposed to be null, but is, and return false to the filter.

This lack of guarantee comes from a number of factors.

For instance, the instance of Visual Studio that this extension is installed in could have a bug introduced by the developers of Visual Studio from one version to another; previously, your original code will work.

But now, hypothetically, let's suppose a newbie developer on the Visual Studio team writes a piece of code that causes nulls to be returned by SVsExtensionManagerService all of a sudden after the local user updates their VS install, and now we're boned.

The new code, as written, won't raise exceptions.

It will, however, be unable to perform the requested functions.

But it won't be this extension's fault.

[astrohart]

@madskristensen Hi Mads,

I am a little new to VSIX programming, I will admit, but I think I have most of the pieces down.

Figure 1. Supported = false being set in the ExportSolutionCommand constructor.

See the Figure 1 above, where I have your code on master displayed, with the line circled as indicated.

Did you mean to do that? I assume that the ExportSolutionCommand.Execute method carries out the process that occurs when I right-click the solution and choose Manage Solution Extensions. Right?

I see where the command is in the .vsct file. And it's displayed on the VS menu and it works (when I click on it, the dialog box for selecting extensions is called up).

My understanding of the MenuCommand.Supported property is that it is supposed to be set to false by an extension author if that command is not supported.

I've really had a hard time coming up with Microsoft docs on this property besides "this is the MenuCommand.Supported property, it's a boolean, and here's how you set it to true in your code." The docs don't say what happens if the property is set to false such as in your code.

As in, is the menu command supposed to be grayed out? Is it supposed to be hidden?

Whatever is intended to happen by that property being set to false, the Manage Solution Extensions... command is still shown and selectable!

Thanks in advance for any help figuring this out.

[astrohart]

My question would be, can I remove the call to set Supported = false on that line?

It appears to not change the behavior either way, unless there is something I am missing.

Best to just get rid of it, to avoid confusion, if that is the case.

[madskristensen]

Supported = false is not why you think it is. And it is required

[astrohart]

Can you please clarify what that is for? So I can provide correct XML docs.

I looked up the Microsoft Docs page for the property but the page is of little help.

The page on Microsoft Docs apparently states that the property exists, that can be either true or false, and shows a code sample to illustrate how to assign it a value of true.

But they do not specify what the values mean.

Was hoping maybe you could provide some insight.

Im not questioning whether your code is correct.

Im just trying to understand what that piece of code does; what effect it has.

[astrohart]

Supported = false is not why you think it is. And it is required

Can you clarify?

I just want to know what setting Supported = false does.

No one is going to change the code to do otherwise.

I am simply wanting to know the effect that that has, so I can document it in the XML documentation correctly.

[madskristensen]

Sorry, I was brief because I was on my phone writing the comment to unblock you. Supported = false means that the visibility of the command is delegated back to the <VisibilityConstraints> in the .vsct file and the UI Context rule used, even after the package is loaded.

[loop8ack]

Hello @astrohart,

I noticed that your pull request, as well as your last commit, were submitted quite some time ago. Nevertheless, I would like to provide some feedback since you put in a lot of effort.

Firstly, I want to acknowledge the value of the project's fundamental idea, and it's unfortunate that it wasn't actively developed further. I have taken over the project from madskristensen and intend to continue its development, incorporating some of my own ideas as well.

Currently, my main goal is to bring the code into a state where I can work with it effectively, including addressing the points you raised in #59.

However, I must mention that I have different perspectives on several aspects of the solution you proposed. Therefore, I will not be merging your pull request, but instead, I will develop my own design that aligns with my vision for the project.

Nevertheless, I would appreciate it if you could review my progress critically, as I believe discussing alternative solutions could lead to better outcomes. Your input and insights are valuable, and I'm open to collaborating on finding the most optimal approach.

Thank you for your hard work and dedication to the project.

[astrohart]

@loop8ack Thank you for the heads up that you are taking over the project from @madskristensen. I also wanted to just let you know that I am actually not finished with all the commits that I am wanting to merge. I had created this PR and I had told Mads that I am committing as I go. Then I got a new job and launched a whole new phase of my career and have not had time to come back to my refactoring effort. I want to ask if you would mind (a) sharing more in-depth what your vision for the project is and (b) allowing me to refactor for SOLIDity and robustness. I've been coding since 1994 so I see myself as a fount of knowledge and experience.

I am also trying to update the Look and Feel (L&F) from the current one which I had devised. I want to move more to a L&F for the dialogs that more closely resembles the GUI for the Create New Project wizard etc. (No title bar, moving the window by dragging anywhere in it, more user-friendly main instruction and other prompts) so that it blends more seamlessly with the Visual Studio UX overall.

I am just having had to back burner this since I had taken my career in some new directions recently.

I would still ask to be retained as Contributor status and I would like to invite you to engage in a dialog with me. I understand every programmer has their own "religious preferences," but, for the sake of argument, I ask that you hear me out as to how to make the code production-ready.

[astrohart]

FYI - this PR is not yet complete.

[loop8ack]

I would still ask to be retained as Contributor status

You are still a contributor, aren't you? You won't lose your status and are welcome to continue contributing. However, you should refrain from continuing your work for now, as it would likely be futile. More details on that below.

(And please excuse the email, my mistake)

I would like to invite you to engage in a dialog with me

How do you envision it? You can certainly create an issue at any time, or I can open the Discussions section? However, personal debates would be a challenge, as my English is not very good and I often need to rely on assistance in writing.

I want to ask if you would mind (a) sharing more in-depth what your vision for the project is and (b) allowing me to refactor for SOLIDity and robustness.

You are welcome to participate, but I would suggest waiting until I have an initial version online. The functionality will remain the same (I won't be developing new features initially), but the structure will be very different.

What I plan to do:

I have a lot in mind, such as:

• Issues, Issues, Issues :D
• Extensions depending on the Visual Studio version, in case teams are working with multiple Visual Studio versions.
• Extensions should be marked as mandatory, so that the solution can only be loaded if these extensions are installed.
• I also have your ideas for better integration into the look and feel of Visual Studio on my to-do list.
• ...

However, these are long-term goals. Before that, I want to completely overhaul the existing code:

Old csproj Format vs. SDK Format

I find the old csproj format really terrible, very cumbersome, and inflexible. I want to use as much code as possible in the new csproj format and also incorporate new language features like nullable reference types. Unfortunately, that's not possible everywhere, but I'll explain more about it shortly.

Shared Projects

Shared projects give me the impression that Microsoft is no longer actively developing this project type. I frequently encounter problems with it, for example, when adding a new file. Here, too, I want to have as much of this code as possible in projects with the new SDK format. Unfortunately, that's not possible everywhere, but I'll explain more about it shortly.

S.O.L.I.D. & MVVM

Of course, I also want to clean up the code and implement the S.O.L.I.D. principles as well as MVVM. In my ideal vision, comprehensive test coverage will also be possible in the long run.

Code Documentation

I will not write code documentation because I believe that the code itself should be self-documenting. However, in cases where the code is not sufficiently understandable without comments, I will certainly write comment documentation.

My current state ...

... is not publicly visible as I am still in the experimentation phase and unsure whether I would like to continue this concept. Currently, it is working with an abstraction around the Visual Studio SDK, allowing most of the code to be in SDK-style (non-shared) projects. Additionally, this approach allows me to centrally handle version conflicts of Visual Studio's DLLs (#70, #71, #75) and easily extend it for future versions.

I'm making good progress at the moment, but I also have a job transition ahead of me, so I will have less time in a few weeks compared to now. I hope to publish a version by then.

[astrohart]

@loop8ack I see you invited me to the project. I clicked the link but then it said "404." Can you please send me a new invite?

Also I have some discussion to share with you:

Old csproj Format vs SDK Format

I am an advocate for that as well, but I say wait until the latest updates can be addressed. The reason being, I prefer to edit the code in VS2019 and I am also more experienced with the .csproj format. I request to wait until the next release can be readied before migrating a complex project such as this one.

Shared Projects

Shared Projects is still a supported format, and, as written, this extension supports VS2017, 2019, and 2022. I agree that there may be a better way to skin that cat; however, I do like, for the moment, how the Shared Projects format can group the code common to all the new versions in just one project. I think that that is efficient.

S.O.L.I.D. & MVVM

I agree completely! That is what I am working on in the present PR. The present PR is not in a complete state, and I ask that you allow me to complete my modifications before taking the code to the next step in your vision.

It needs a lot of refactoring because it is neither SOLID nor MVVM. Let me finish all my commits and please merge it, and then we can carry it forward. At least -- in this PR. I want you to not discount what I am doing until you've seen the full monty.

There needs to also be more robustness and fault tolerance, in my opinion. The code should be made production-ready.

Window UI/UX Updates

Figure 1 Vision for the Import/Export Extensions window.

The image above illustrates what I mean by the UI/UX of the Import/Export Extensions window. Basically, we are bringing its look and feel into alignment with the Visual Studio Create New Project window and such. Doing away with the title bar and making a label with enlarged text for the title is the new trend. Making the border resizable and adding only a Maximize button in the corner is the way to adapt the window's functionality. Resizing the border and maximizing the window are things the user can do in case the list of extensions has a very large number of items. To move the window on their screen, the user should be able to click and drag the mouse anywhere in the window's client area.

Code Documentation

I strongly disagree. XML documentation should be on all classes/methods. After all, what if others wish to collaborate and using a ToolTip in the IDE (powered by XML docs) that explains what a class or interface is for, or what a method parameter is for, is useful when someone else is programming this API?

Plus, there is a library called Vsxmd that "eats" the XML docs and spits out README.md files during the build. This would make this repo self-documenting.

I believe that the code itself should be self-documenting

This is not to be unkind in any way, but I find a lot of developers believe this. But people on the business side or maintainers of the code when you leave the project disagree -- they are left in the lurch. I also have 30 years of S/W development experience behind me, so hear me out: the code normally does document itself --- to a trained pair of eyes such as a developer with much experience like us. However, should novice developers want to contribute, they might get confused. Furthermore, documentation is needed to say what values are and are not acceptable for parameters and to document what assumptions went into code, and what happens if bad values are passed in. Edge cases, assumptions, and failure modes are hard to "self-document" in code. Having in-code documentation mitigates those issues.

BTW -- I'd love to be the one to take over the development of this extension. I am surprised, actually, that @madskristensen did not transfer it to me initially. If you feel you want to move on to other projects, I'd be happy to take over. Please don't take this as me trying to do anything wrong, or untoward -- you can see for yourself the long history I've had with this project. I just am offering, this in case you come across as not having the bandwidth to continue.

[loop8ack]

I'd love to be the one to take over the development of this extension. I am surprised, actually, that @madskristensen did not transfer it to me initially.

The project has been stagnant for a long time, and you haven't worked on your pull request for over a year. I understand the reason behind your break, but you must also understand why I took the initiative and want to carry it forward now.

I have already made significant progress, and my changes are too extensive to merge or wait for your pull request at this point. Therefore, I won't wait, but I will create a branch in a few days that includes the new structure. We can discuss it further then.

Old csproj Format vs SDK Format

I already have a concept in place that allows using the SDK format. Unfortunately, I need an abstraction for that since the versions of the four dependencies are not compatible with each other. However, the advantage is that it enables the use of unit tests.

Shared Projects

Shared Projects is still a supported format

I know, but nevertheless, it hasn't been actively developed for a long time. Hence, my assumption that Microsoft has discontinued its development.

I do like, for the moment, how the Shared Projects format can group the code common to all the new versions in just one project. I think that that is efficient.

Of course, they are definitely efficient, but currently, I don't see a future for them. I use them as well because the only other option is file links, which is even worse.

Window UI/UX Updates

I like your design. But I have already redeveloped the window so that it works cleanly with MVVM. Currently, it only replicates what the old version could do; new features or a new UI/UX design will come later.

Code Documentation

I strongly disagree. XML documentation should be on all classes/methods. After all, what if others wish to collaborate and using a ToolTip in the IDE (powered by XML docs) that explains what a class or interface is for, or what a method parameter is for, is useful when someone else is programming this API?

Most of the time is spent reading code, not writing it, so the most important thing is that the code is self-explanatory. The problem with comments is that, in my experience, they tend to be neglected over time, and incorrect comments are worse than having none at all.

Therefore, I prefer not to rely on comments and instead focus on making the code understandable. If necessary, I will adjust the code to make it clearer. If there is no other option, I will add a comment to explain the function and also clarify why it is structured that way.

But people on the business side or maintainers of the code when you leave the project disagree -- they are left in the lurch. [...] However, should novice developers want to contribute, they might get confused.

I have had different experiences. I was expected for years to write code that doesn't need comments to be understood. And I have found that inexperienced developers, in particular, are often confused by outdated comments and tend to neglect them themselves.

But this debate is old, and there will always be two groups. It is therefore pointless to discuss it further here.

[loop8ack]

@astrohart

I have pushed my redevelopment. You can find the code in the following branch: https://github.com/loop8ack/ExtensionPackTools/tree/Redevelopment

I didn't put a lot of effort into making the xaml code clearer, as I will probably change some things there for future features. Also, I have only done smoke tests with this version, so it is possible that there are still bugs.

[loop8ack]

Hello @astrohart,

are you still interested in working on this project? I was hoping for a constructive discussion about my design, especially because I am taking an unusual approach with Facade Pattern.

[astrohart]

Yes I'd like to, in principle but I'm having trouble coming to a meeting of the minds in terms of S/W Dev approach. We seem to be on opposite sides of a religious debate in some areas.

[loop8ack]

Which areas do you mean? The question about comments?

[loop8ack]

Ok, then I close the pull request. Of course, you are still invited to participate, and you will keep your contributor status.

[astrohart]

Please don't close this PR. I did want to respond when you reached out earlier about still being open to my contribution and I want to ask you to hear me out and let me finish this PR before you close it. Thank you. I've just been so fucking slammed lately, and I also lost the email where you contacted me earlier. Your GitHub handle is hard for me to remember, for some reason. Thank you.

[loop8ack]

Alright, I have reopened the Pull Request.

It is important to me to have a review of my branch, as I have taken a few unusual paths. Before permanently adopting my code, I am interested in hearing more opinions on it.

[astrohart]

Thanks for being receptive to my message, and open to continue to work with me! :-)

[loop8ack]

Please don't put too much effort into it :) You mentioned that we might have differing opinions, so it would be better if, for now, you just described what you would do without actually implementing it. That way, we can discuss it, and you won't have to spend too much of your limited time on it.

Also, I just pushed a larger commit. I have revised the UI integration to make it more flexible, allowing for features like a loading bar in the window. In my opinion, it looks significantly better now and provides more flexibility for future changes.

And after a second commit, the Visual Studio Facade is distributed via Dependency Injection