command-log-mode icon indicating copy to clipboard operation
command-log-mode copied to clipboard

Doesn't follow package etiquette (esp. keys bindings)

Open WorldsEndless opened this issue 9 years ago • 5 comments

I really like what this package does, but it makes some very bad choices with key binding. I'll point out two examples: C-c o is bound to command-log-mode-key-binding-open-log. But extensions should never bind C-c letter keys.

An equal problem is with it being a global binding; it should only be bound when the mode is active. I've been having a number of problems because of the C-c o binding.

Thanks again for excellent functionality; these are just a few things that will improve the plugin.

WorldsEndless avatar Mar 01 '15 01:03 WorldsEndless

+1

stardiviner avatar Oct 01 '15 01:10 stardiviner

The source code:

(eval-after-load 'command-log-mode
  '(when command-log-mode-key-binding-open-log
    (global-set-key
     (kbd command-log-mode-key-binding-open-log)
     'clm/toggle-command-log-buffer)))

This will set global keybinding, and override user defined keybinding prefix [C-c o]. I suggest at least don't set option command-log-mode-key-binding-open-log keybinding, by default set to nil. Let user decide what keybinding to do.

Also, user usually don't need to toggle *command-log* buffer show/hide. Like me, I just enable command-log-mode or disable it. let it show buffer automatically.

stardiviner avatar Oct 01 '15 02:10 stardiviner

What about this PR?

stardiviner avatar Oct 26 '16 03:10 stardiviner

Hi @stardiviner, I have forked this project and integrated your suggestion in the PR in my fork of the project. I'd like to see the changes integrated here but there has not been any activity since April 2013.

pierre-rouleau avatar Nov 27 '20 16:11 pierre-rouleau

If you want to maintain this project, I suggest you following methods:

  1. send email or other ways to contact the project author, then ask for permission to add you as collaborator.
  2. Or ask for permission to let MELPA use your fork to replace MELPA recipe.

stardiviner avatar Nov 28 '20 01:11 stardiviner