Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

M-backspace does not effectively delete words in ansi-term #8

Open
wizmer opened this issue Oct 3, 2017 · 10 comments
Open

M-backspace does not effectively delete words in ansi-term #8

wizmer opened this issue Oct 3, 2017 · 10 comments

Comments

@wizmer
Copy link

wizmer commented Oct 3, 2017

Description :octocat:

M-backspace visually delete a word in ansi-term but when Enter is pressed the deleted word is still part of the command.

Reproduction guide 🪲

  • Start Emacs
  • M-x ansi-term
  • Type "ls"
  • Delete it using M-backspace
  • press Enter

Observed behaviour: 👀 💔
The "ls" command is performed even it had been deleted

Expected behaviour: ❤️ 😄
The "ls" command should not be performed.
The problem is that M-backspace is overriden by clean-aindent-mode and set to (clean-aindent--bsunindent ARG)

Maybe clean-aindent should always be disabled when being in term-mode ?

PS: The issue was originally opened here: syl20bnr/spacemacs#9564

wizmer pushed a commit to wizmer/clean-aindent-mode that referenced this issue Oct 3, 2017
wizmer pushed a commit to wizmer/clean-aindent-mode that referenced this issue Oct 3, 2017
@pmarinov
Copy link
Owner

pmarinov commented Oct 5, 2017

Thank you for submitting your fix.

Isn't it better if the entire function clean-aindent--bsunindent(arg) checks for term-mode and bypasses its present functionality?

I think your verification snippet needs to be immediately after (interactive "p"), because the entire functionality of Backspace Unindent has no meaning in term-mode.

I propose you change it as:

(interactive "p")
  (if (eq 'term-mode major-mode)
      ;; Special case, bypass functionality on 'term-mode
      (term-send-raw-meta)
    ;; else: all other editing modes are OK
    (if (not (clean-aindent--inside-indentp))
    etc...

What do you think?

@wizmer
Copy link
Author

wizmer commented Oct 5, 2017

I don't know exactly what the whole mode is doing (I discovered it while trying to understand why M-backspace was not working), but I think it would even make sense to disable the whole mode in term-mode
I don't know if this is doable why keeping the mode as :global

@pmarinov
Copy link
Owner

pmarinov commented Oct 5, 2017

Yes, you are right, disabling the entire mode inside "term-mode" is best.

I added this to my "init.el" to test the idea:

(defun term-mode-customize()
  ;; Clean AIndent Mode conflicts with functionality of "term-mode"
  (clean-aindent-mode -1))
(add-hook 'term-mode-hook 'term-mode-customize)

Then starting new M-x term has Alt-Backspace work as originally intended without the conflict.

Can this be added as part of the setup of spacemacs?

I can also add this snippet to the documentation (README) of clean-aindent-mode.

@wizmer
Copy link
Author

wizmer commented Oct 8, 2017

Ah ! I finally found the correct solution !
We just need to change:
(define-key clean-aindent-mode--keymap (kbd "M-DEL") 'clean-aindent--bsunindent)
to
(define-key clean-aindent-mode--keymap [remap backward-kill-word] 'clean-aindent--bsunindent)

That's it :)

@aaronjensen
Copy link

Hm, can you explain why that works @wizmer? I came here to file the same bug. I fixed it as @pmarinov suggested, but, because it's a global minor mode this disables it entirely whenever you first enter a term, so that's not ideal.

@aaronjensen
Copy link

I think I understand it now, remap actually causes the command to take over whenever backward-kill-word would have been called. That's super helpful to know and does seem like the right solution to this.

@pmarinov
Copy link
Owner

I like this idea:
(define-key clean-aindent-mode--keymap [remap backward-kill-word] 'clean-aindent--bsunindent)

I will test that myself now.

@pmarinov
Copy link
Owner

It works. I tested with emacs -q and manually (load-file "new-clean-aindent-mode.el") and activated the mode to make sure in a clean environment that the idea is good.

Now, @wizmer if you submit a pull request with this change I will merge it. Thank you for this very nice solution.

@aaronjensen
Copy link

@pmarinov #9

@pmarinov
Copy link
Owner

It has been merged. Melpa will pick it up automatically, let's see how it goes.
:-)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants