rougier / nano-emacs

GNU Emacs / N Λ N O - Emacs made simple
GNU General Public License v3.0
2.5k stars 191 forks source link

[Bug] Modeline and calendar and org-mode dates S-arrows navigation issue #131

Closed wakatara closed 1 year ago

wakatara commented 1 year ago

Description

S-arrowkeys don't work for navigating dates when triggering calendar buffer in modeline. This is the standard way of using calendar for most people (and quite a usability issue for most people muscle-memory wise.)

To reproduce

  1. Call calendar from an org document (C-. etc) to insert a date (or change a deadline or scheduled date)
  2. modeline pops up attractively highlighting the date (sooo nice)
  3. Attempt to use S-rightarrow (or leftarrow or downarrow or uparrow) to change dates
  4. Does not work

Expected Behaviour

Should be able to navigate dates in the modeline calendar minibuffer with S-arrowkeys

Actual Behaviour

Notes

Discussed in reddit here: https://www.reddit.com/r/emacs/comments/zhy3v7/strange_nano_emacs_date_picking_behaviour_sarrows/

wakatara commented 1 year ago

Update: this works with all S-arrowkey functions highlighting any org-mode date. So, if you are in a TODO Deadline even without the mini-calendar popup (ie. just in the region of the org-mode date and try to increment or decrement it via the S-right or S-left etc, you get the same error message in an sort of org-mode document.).

wakatara commented 1 year ago

Btw, I disabled nano-modeline completely to test this and it had no effect. The calendar popups and text dates within org-mode docs still get the same issue with you try to S-Arrow on them. Will do a bit more testing on it...

wakatara commented 1 year ago

@rougier OK, I've discovered where this issue is caused.

In nano-emacs/nano-defaults.el line 96 You have a small one line stanza here which overrides shift-direction for window navigation

;; Navigate windows using shift+direction
(windmove-default-keybindings)

windmove is actually mentioned in the the conflict document for org-mode. To get windmove to operate in this fashion outside of areas where org-mode would normally be using S-arrow, it suggests the following change.

;; Make windmove work in Org mode:
(add-hook 'org-shiftup-final-hook 'windmove-up)
(add-hook 'org-shiftleft-final-hook 'windmove-left)
(add-hook 'org-shiftdown-final-hook 'windmove-down)
(add-hook 'org-shiftright-final-hook 'windmove-right)

Or whether kiilling windmove altogether makes more sense (or switching it to alt-arrow? Not sure if that conflicts with anything else in org-mode though.).

lemme know if you'd like me to take a shot at correcting this with the above (not even sure how many people use S-arrow to move windows) and making a pull request. I have yet to make any contrib to emacs package code yet though (more a go, ruby, pthon, typescript guy) but willing to take a shot especially if you're correcting other bugs to get out a new release over the holidays or something.

wakatara commented 1 year ago

I forked nano via straight and commented out windmove-default-keybindings and it immediately corrects the errant behaviour org-mode expects for the calendar popup and any date written in text in the format <2022-12-19 Mon>

(straight-use-package
 '(nano-emacs :type git :host github :repo "rougier/nano-emacs"
              :fork (:host github
                   :repo "wakatara/nano-emacs")))

I guess the quesiton is now whether we drop windmove support (not sure how many people use the S-arrow for window movement. I always just have 2 windows open on my laptop) or a diff set of keys to keep it as per the conflict doc for org-mode.

rougier commented 1 year ago

Thanks for the initial bug report and fix. Maybe safest move is to drop (windmove-default-keybindings) (and I'm personally not actually using). Can you make a PR?

wakatara commented 1 year ago

Sure! Happy to help out. I'm really enjoying nano (so, again... thanks for putting all the work into creating it.).

Lemme eliminate windmove from the setup completely and make the pull request. Will try to get to it this evening here in TH.

wakatara commented 1 year ago

PR now with you for merge. Will leave it to you to close this ticket though.

rougier commented 1 year ago

Merged! Thanks.