emacs-php / php-mode

A powerful and flexible Emacs major mode for editing PHP scripts
GNU General Public License v3.0
583 stars 118 forks source link

(define-key map [tab] 'indent-for-tab-command) is a bug #688

Open phil-s opened 2 years ago

phil-s commented 2 years ago
    ;; Use the Emacs standard indentation binding. This may upset c-mode
    ;; which does not follow this at the moment, but I see no better
    ;; choice.
    (define-key map [tab] 'indent-for-tab-command)

That last line should be:

    (define-key map (kbd "TAB") 'indent-for-tab-command)

I can see that the original code correctly used TAB (\t) instead of <tab> ([tab]) and it was later changed (commit 3699acab27c92044f4c7b618c75b9194fd005e0d); but I strongly suspect that change was made for purely aesthetic purposes, as it's a bug.

Terminals don't send <tab> events; they only deal with TAB. This is why Emacs (a) translates <tab> to TAB, and then (b) uses TAB consistently for binding keys (and certainly the referenced c-mode-map binds TAB and not <tab>). This convention ensures that GUI and terminal Emacs always do the same thing.

With the current code, C-h k TAB tells us:

zonuexe commented 2 years ago

@phil-s Thanks for your suggestion.

I learned that [tab] doesn't work literally by eval the code below.

(local-key-binding [tab] (lambda (interactive) (message "TAB!")))

It looks like he was trying to run indent-for-tab-command directly, avoiding c-indent-line-or-region, but it didn't seem to work.

  (substitute-key-definition 'indent-for-tab-command
                 ;; XXX Is this the right thing to do
                 ;; here?
                 'c-indent-line-or-region
                 c-mode-base-map global-map)

I haven't checked the behavior in old Emacsen, but the code hasn't changed from Emacs23 to the present.

Delete these lines as they are just verbose definitions.

phil-s commented 2 years ago

I'm confused by the reply, but the correct fix for php-mode.el is what I've shown in my initial post, and if you want to sort it in your own config you could do this:

;; Bug fix for https://github.com/emacs-php/php-mode/issues/688
(with-eval-after-load "php-mode"
  (when-let ((cmd (lookup-key php-mode-map [tab])))
    (define-key php-mode-map [tab] nil)
    (define-key php-mode-map (kbd "TAB") cmd)))

Note that local-key-binding isn't for binding keys.