madskristensen / Tweakster

A Visual Studio extension
Apache License 2.0
240 stars 23 forks source link

Reset Zoom command cannot be disabled and shortcut cannot be removed #38

Open yannduran opened 3 years ago

yannduran commented 3 years ago

Time Savers has used CTRL-0 to initiate a solution rebuild since 2016-10-16. It's entrenched in my muscle memory, and potentially also muscle memory of the almost 4K developers who have installed Time Savers and may use it all the time like I do.

I thought I'd just go ahead and remove the shortcut manually, but the command doesn't even show up in Keyboard Options. It looks like when the CommandWellOnly command flag is added to a button that any key binding for it cannot be searched for, and therefore cannot be removed.

When searching for the shortcut I tried both ResetZoom and ZoomReset (see below), as well as looking in the results for just Reset and Zoom.

The command also can't be disabled as there's no setting to do that.

CTRL+NUM 0 is available and might be a better shortcut for this command.

Also, while I was investigating, I also found that the LocCannonicalName appears to be incorrect.

      <Button guid="guidCommands" id="ResetZoom" priority="0x0100" type="Button">
        <CommandFlag>CommandWellOnly</CommandFlag>
        <Strings>
          <ButtonText>Reset Zoom</ButtonText>
          <LocCanonicalName>.View.ZoomReset</LocCanonicalName>
        </Strings>
      </Button>

It doesn't matter for this shortcut problem, it's just an inconsistency. The id is ResetZoom, and the button text is Reset Zoom. So it follows that the canonical name would be .View.ResetZoom, not .View.ZoomReset.

I'm happy to create a pull request that addresses any/all of the points below if you agree with what I've proposed and you'd like me to go ahead and do that.

  1. Change shortcut to CTRL+NUM 0
  2. Add UI to disable the command, and implement it
  3. Fix the inconsistency of the canonical name in the VSCT file
madskristensen commented 3 years ago

I can remove and reassign the shortcut key with no issues. Does Time Savers not use the regular keybinding system? Is anyone else experience this?

The command is called View.ZoomReset to align it with the other existing Zoom commands like View.ZoomIn and View.ZoomOut.

yannduran commented 3 years ago

No Time Savers just uses the regular keybinding system. I wasn't actually talking about removing the Time Savers shortcut though, I wanted to remove the Reset Zoom one but couldn't find it, and therefore couldn't remove it.

But I've just had a look again, and I can now both find and remove the CTRL+0 binding for View.ZoomReset. My Visual Studio instance has been doing a couple of weird things lately, so maybe that's why I couldn't find it, I don't know. Very weird!

Thanks for the explanation for why the command to reset zoom is called ZoomReset instead of ResetZoom.

I still have a concern that there may be Time Savers users who install Tweaks and find CTRL+0 now no longer does what it used to and think there's something wrong with Time Savers. They may not know how to remove the new keybinding that Tweaks adds. Seeing that Tweaks in still in Preview, and Time Savers has used CTRL+0 for a long time I thought that maybe you might have been able to change the keybinding that Teaks uses.

But if you want to keep CTRL-0 for ZoomReset in Tweaks I'll close this issue as my main concern was that I wasn't able to find or remove it when I tried to do it.

madskristensen commented 3 years ago

My original shortcut was actually Ctrl+0,Ctrl+0 but that would also interfere with Time Savers. The reason Ctrl+0 is so powerful for this, is that it's what all other apps use for resetting zoom, including all browsers. So it's a shortcut you kinda already know what will do.

yannduran commented 3 years ago

Time Savers doesn't use CTRL-0, CTRL+0. I just double-checked both my currently installed instance and the keybindings in the VSCT file to make sure.

What did you think it was used for?