dwmkerr / sharpshell

SharpShell makes it easy to create Windows Shell Extensions using the .NET Framework.
MIT License
1.5k stars 260 forks source link

RFC ServerManager - List of Improvements #217

Open Countryen opened 5 years ago

Countryen commented 5 years ago

Hey there, since I try to help out here in the forums I tend to make many prototypes. I use the tool "Server Manager" - downloaded and installed from the releases.

Here are a few thoughts/issues I noticed with the tool. Wanted to discuss them with you before trying to resolve them with pull requests on my own. None of them are critical, more improvements than corrections.

I know the list is quite long but I didn't want to open 20 issues. Also, these are my humble suggestions/onions - be welcomed to disagree.

Would love to get the tool working smoothly for further development and testing. tyvmia for any contribution :)

My Setup:

Legend / Definitions

1 General Topics & Questions

1.1 Should you still use Server Manager?

As there is the old-school way with GACUtil and Regasm, and now the prefered way with SRM, I wonder if the SM still is part of the "officially supported toolchain". I can't find any documentation for the SM (tbh, I didn't search codeplex/codeproject).

1.2 Download Warnings

Short: Browser/OS warnings when downloading/using SM. Long: Downloading SM, the browser (Chrome) and when starting even the OS (Windows) warns me not to use it. Note: Maybe only occurring when freshly published. Note: Also applies to downloading SRM. Edit: November 6, 2018 - 18:13 (UTC): Checked, GitHub automatically signs and verifies Releases so everything is good - still, application signing could be beneficial for OS/Browser warnings.

2 GUI Improvements / Minor Improvements

2.1 Outdated Help Section with old links

"Help > About" still directs the user to the old project on codeplex. Should be GitHub now. Note: Also there is a missing ':'. Note: The full "Help" section needs to be updated.

2.2 "Attach Debugger" prompt can easily be misunderstood as "something went wrong"

Suggestion: Change the prompt, add additional information, add documentation. Long: If you click on "Attach Debugger" you are prompted by VS JIT Debugger that "An unhandled Microsoft .NET Framework exception occured in ServerManager.exe [20088]. ...". I guess this is because we WANT to attach the debugger and it's the default message. It is a bit misleading. I don't know if you can change it at all, though. Note: Maybe adding a "Do you really... (with an optional "never ever ask me again") would be good. Note: By the way, what exactly is this button used for? Note: Maybe change the icon, too? I always mistake it for a config-button.

2.3 Minor changes (little goes big)

3 Feature Requests

3.1 No simple "Remove/Unload Server" option?

You can add Servers via File > Load Server, but not remove them? (Also related to Nr. 6, I guess that's currently the only way to remove them)

3.2 Docs: Addtional Troubleshooting: Adding NirSoft Toolchain

ShellExViewer, ShellMenuView the other NirSoft tools can help you diagnose problems with your Servers. Maybe we could add it as "Addional Developing Tools" or something (like the DebugView from SysInternals already is). https://www.nirsoft.net/utils/shexview.html -> Show, Change, Disable, Enable and open in Registry of all existing/registered ShellEx, Servers, etc. -> Helped me a bunch of times.

3.3 ContextMenu-Actions for the list of Servers

Would be nice to be able to click on a Server and (de-)install/(de-)register, test, remove or refresh a Server manually. Note: Maybe add multi selection operations.

3.4 Search registry for Servers

SM could (on button click) search the windows registry for registered/installed servers and add/update them to/in the list. Workaround: Find Server in registry shexview, find file of Server, add to list. Would be helpful when "losing" your extension.

3.5 Open in registry for Servers

As seen with the shexview, would be nice if you could click on a server and say "Open in Registry" and then choose "CLSID" etc. Jumping to the registry instead manually searching for the keys yourself. Note: Also add an option to export the full "used" registry data (for diagnose and documentation).

3.6 Open in file browser for Servers

Same as above - for opening the file path of the Server's DLL-file in file browser. (for the lazy folks)

3.7 Configure SRM from SM (Added by Edit: October 28, 2018 - 21:55 (UTC))

Would be cool to display and change the global srm config options via SM (GUI).

4 Issues

4.1 Drag-&-Drop Loading of Servers not working?

Suggestion: Fix or remove information at bottom. Long: At the bottom it says: "Drop SharpShell Server DLLs onto the list to analyse them". I can't drop files there (no drop event, not even a cursor). Isn't it supposed to load the server on drop of the dll file?

4.2 Details for Servers do not refresh automatically

Suggestion: Fix by refreshing List and Details on change of Server. Changing a Server in the list via "Server > xyz" doesn't update the information on the right. You need to manually click the Server in the list again to refresh it. Note: The Server is still highlighted/selected, though.

4.3 Issues when renaming/removing the Server while the program is open

Suggestion: Identify & fix the crash, show/manage (unloaded but still "known") Servers. The application crashes when you delete/rename a (loaded) Server-file while ServerManager is opened. On reopening ServerManager says the Server "wouldn't be a SharpShell Server" and is not listed anymore. The message will then appear every time you open SM. You need to run "Tools > Clear most recently used servers" to get rid of it.

4.4 SM blocks loaded Server's VS projects from recompiling

Suggestion: Not blocking the DLL/Server for better dev experience. I compile my project with VS2017, then I load that server from the /bin/debug/ path. As long as the ServerManager is open, you can't recompile your project. Following error message from VS (original and translation) "obj\Debug\WEX.Prototype12.dll" konnte nicht in "bin\Debug\WEX.Prototype12.dll" kopiert werden.[...] Die Datei wird durch "ServerManager.exe (20476)" gesperrt. "obj\Debug\WEX.Prototype12.dll" couldn't be copied to "bin\Debug\WEX.Prototype12.dll".[...] The file is blocked/locked by "ServerManager.exe (20476)". Workaround 1 = Closing the ServerManager. Workaround 2 = Loading the DLL not directly from the output bin (copy/clone). Problem is, both solutions are taking longer than needed/wanted. Why does that happen? Guess: It's the TestShell. Could be optional (not using the TestShell = Not blocking the assemblies?) Note: You can rename the file just fine, only VS doesn't want to do it.

4.5 "Tools > Clear most recently used servers" doesn't refresh the list of servers immediately

Suggestion: Fix by refreshing the list or closing the SM or giving additional information. Clicking on the button, apparently nothing happens. When you close the SM tool and reopen all the Servers are gone. Should be immediately or more inoformation.

4.6 (De-)Installation/(De-)Registration problems

Note: Maybe not just a problem of SM but rather of SRM? I noticed some weirdness around (de-)installing/registration of Servers. I think it needs some attention (to keep the procedure well documented and the registry clean). (After each action I note which keys I found in the registry, searching (F3) for CLSID or Name of my Prototype) SHELLEX = Computer\HKEY_CLASSES_ROOT*\shellex\ContextMenuHandlers\... PID = Computer\HKEY_CLASSES_ROOT\... CLSID = Computer\HKEY_CLASSES_ROOT\CLSID\...

Why no PID from "Install" when you specify the bitness? Also, doesn't "Register" mean like RegAsm and "Install" mean GACutil? I couldn't find the Assembly in my GAC at all (it's always codebase). Paths I searched: C:\Windows\Microsoft.NET\assembly* & C:\Windows\assembly* Note: Didn't analyze it in detail, but it's the same for x86.


Edit: October 28, 2018 - 21:55 (UTC): added FR "Configure SRM from SM" Edit: November 6, 2018 - 18:13 (UTC): updated "Download Warnings" Edit: November 12, 2018 - 21:13 (UTC): added numbers to the headers"

dwmkerr commented 5 years ago

This is excellent, thanks @Countryen. Once I've got a handle on the property sheet issues I'll look into this in detail. Until then feel free to add more thoughts as you come across them!

Countryen commented 5 years ago

Notes on:

No simple "Remove/Unload Server" option? SM blocks loaded Server's VS projects from recompiling "Tools > Clear most recently used servers" doesn't refresh the list of servers immediately Issues when renaming/removing the Server while the program is open

Apparently MEF is used and loads the Assemblies. MEF can not unload Assemblies dynamically so this will always be a problem. A solution (according to some research) would be to load the Servers into a different AppDomain and unload this AppDomain prior to compiling.

Maybe a simpler way would be to just load a ShadowCopy of the actual Server's DLL file (copy it to %temp% or smth.). Sure, registration must use the actual path, but this shouldn't be an issue :)

A "reaload" would be necessary, though. Could still need the AppDomain thing, I guess. Maybe you can add a seperate AppDomain for each Server? Then single reloading would be a possible.

I wonder if you can "reload" the same MEF-Exports and just replace ServerEntry.Server with the new Export's instance and it works fine?

Yet again, if you don't use the test-shell / explorer in SM, you don't need MEF. Just using LazyLoading yourself should be enough (releasing the DLL right afterwards).

dwmkerr commented 5 years ago

@Countryen MEF is never going to be a viable solution. The problem is that any server compiled against a different version of SharpShell to the one which is being used in a tool is not going to load, so it just makes the whole thing way too brittle. I'm going to raise an issue to simple remove MEF, and instead use reflection to attempt to load the correct type an inspect it dynamically. Does this make sense? I.e. I'll look for types which implement SharpShellServer, regardless of version

Countryen commented 5 years ago

@dwmkerr thanks for your insight. I agree that using LazyLoading / Reflection should be prefered and would resolve multiple issues. But as it's a bigger thing I will contribute to your new issue and "remove" the points from this list of issues.