On Sun, 10 Nov 2013, Mark Walters <markwalters1009@gmail.com> wrote: > Hi > > David found a bug in the this remap/help series. He has a global > keybinding of "C-c s" for notmuch-search and this causes help in > tree-mode to hang. > > I have mostly diagnosed this: the problem comes that all the construct > help routines are inside a string-match/replace-match pair. Somewhere > in these routines the match-data is being stomped on (but I have to > admit I am not sure where). > > In any case putting the construct help routines inside a > save-match-data seems to fix it. > > This version is a bit ugly: I am not sure of the best way to deal with > the save-match-data macro. (I think it is best to have it round > everything that happens between finding the match and replacing the > match to avoid anything similar in future). > > This applies on top of the parent series. > > Any comments gratefully received! > > Best wishes > > Mark > > --- > emacs/notmuch-lib.el | 17 ++++++++++------- > 1 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el > index 7b8acb3..e98e073 100644 > --- a/emacs/notmuch-lib.el > +++ b/emacs/notmuch-lib.el > @@ -305,13 +305,16 @@ prefix argument. PREFIX and TAIL are used internally." > (defun notmuch-substitute-command-keys (doc) > "Like `substitute-command-keys' but with documentation, not function names." > (let ((beg 0)) > - (while (string-match "\\\\{\\([^}[:space:]]*\\)}" doc beg) > - (let* ((keymap-name (substring doc (match-beginning 1) (match-end 1))) > - (keymap (symbol-value (intern keymap-name))) > - (ua-keys (where-is-internal 'universal-argument keymap t)) > - (desc-alist (notmuch-describe-keymap keymap ua-keys keymap)) > - (desc-list (mapcar (lambda (arg) (concat (car arg) "\t" (cdr arg))) desc-alist)) > - (desc (mapconcat #'identity desc-list "\n"))) > + (while (string-match "\\\\{\\([^}[:space:]]*\\)}" doc beg) ;; matches \{not-space} > + (let ((desc > + (save-match-data > + (let* ((keymap-name (substring doc (match-beginning 1) (match-end 1))) > + (keymap (symbol-value (intern keymap-name))) > + (ua-keys (where-is-internal 'universal-argument keymap t)) > + (desc-alist (notmuch-describe-keymap keymap ua-keys keymap)) > + (desc-list (mapcar (lambda (arg) (concat (car arg) "\t" (cdr arg))) desc-alist)) > + (desc (mapconcat #'identity desc-list "\n"))) > + desc)))) Oof, what an annoying place to lose the match data. My one suggestion would be to put (mapconcat #'identity desc-list "\n") as the body of the inner let, rather than binding `desc' and then immediately evaluating to `desc'. That would remove some redundancy, and it's already clear from the outer let that the result of the mapconcat is named `desc'. > (setq doc (replace-match desc 1 1 doc))) > (setq beg (match-end 0))) > doc)) > -- > 1.7.9.1 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch