Code Review: Fixes for VS dialog bugs

Oct 5, 2010 at 6:38 PM

* Added an uninstall button to the dialog
* Unwrap package feed exceptions so it displays the actual exception instead of a vague TargetInvocationException
* Issue tracker #160: "Available package sources” label is missing a colon
* Issue tracker #105: Feed UI doesn't allow copying the URL.

Oct 5, 2010 at 9:08 PM

If I’m not wrong, your change will show the Uninstall button in both the Installed tab and the Online tab. I think we only want to show it in the Installed tab. For the Online tab, we show a checkmark image like we currently do. We should mimic the same behavior as the Extension Manager dialog of VS. 

For the other files, why do you rename ‘extension’ to ‘package’? It introduces inconsistency with existing code. I’d suggest that you either follow the existing convention and use ‘extension’, or rename everything to ‘package’ (which requires much more effort).


I don’t see where you fix #160 and #105. Did you forget to include any files?

Oct 6, 2010 at 2:17 AM

Sorry about that, those files were missing. Also changed all the "extension" used in the context of Packages to packages.

Oct 6, 2010 at 6:05 AM
Edited Oct 6, 2010 at 4:51 PM




Oct 6, 2010 at 4:51 PM

Why are you adding a Context menu to the Options page? I don't think that's what the bug is about. The listbox already works with Control + C, so there is no need to add the context menu.

Oct 6, 2010 at 5:04 PM

Didn't realize that bug was already fixed. On the other hand, Right Click --> Copy isn't half bad.

Oct 6, 2010 at 5:08 PM

If you already have the code written, then it's fine to check it in. But we need to make sure we don't add complex UI in the future unless it's necessary.