* bug#4820: 23.1.50; [PATCH] todo-mode.el @ 2009-10-28 12:20 ` Stephen Berman 2009-10-31 2:30 ` bug#4820: marked as done (23.1.50; [PATCH] todo-mode.el) Emacs bug Tracking System 2009-11-03 22:26 ` bug#4820: 23.1.50; [PATCH] todo-mode.el Stefan Monnier 0 siblings, 2 replies; 3+ messages in thread From: Stephen Berman @ 2009-10-28 12:20 UTC (permalink / raw) To: emacs-pretest-bug [-- Attachment #1: Type: text/plain, Size: 2909 bytes --] Here are two bugs in todo-mode.el; a patch is attached. 1. emacs -Q with no existing file as the value of todo-file-do 2. M-x todo-show 3. Type `e' (todo-edit-item) => Args out of range: 60, 61 This is because there is no todo item, so when todo-edit-item calls todo-item-string, todo-item-start returns (point-min), which is 61, and todo-item-end returns (1- (line-beginning-position)), i.e. 60, which raises the args-out-of-range error on buffer-substring in todo-item-string. The attached fix to todo-edit-item checks whether there is a todo item to edit and throws an error if not. This is consistent with other Todo mode commands, e.g. todo-{raise, lower, delete, file}-item. 1. emacs -Q 2. M-x todo-show 3. Type `i' and at the prompt a todo item, e.g "test", then RET to display the item in the TODO buffer in the category "Todo"; note that narrowing is in effect, as required by Todo mode. 4. Type `t' to display the Todo top priorities list in a window below the window displaying the TODO buffer. 5. In the TODO buffer type `E' on the item to put it in Todo Edit mode in the buffer *TODO Edit*. 6. Type C-x k to kill *TODO Edit* and return to the TODO buffer, which remains unchanged. 7. In the TODO buffer type `t' again. => Now narrowing is no longer in effect in the TODO buffer. This is because todo-top-priorities, although it calls widen within save-restriction, subsequently calls set-buffer, with the result, after the call to make-indirect-buffer in todo-edit-multiline (step 5 above), that narrowing is not restored. The causal chain here is not quite clear to me: in (elisp)Current Buffer there is a warning to use set-buffer within save-current-buffer or save-excursion to guarantee restoration, and in fact in todo-top-priorities set-buffer is within save-excursion, but that is outside of the save-restriction. If the order of save-excursion and save-restriction is switched, then narrowing is correctly restored after step 7 above; however, this order is discouraged in (elisp)Narrowing. But putting save-current-buffer between save-restriction and set-buffer also restores narrowing, so this is what the attached patch does. (I don't know why the failure to restore narrowing is conditioned by make-indirect-buffer; I asked about this on emacs-devel but have not yet gotten any response.) In addition, the use of save-excursion appears to be gratuitous here: point is already moved by the preceding call to todo-show, and subsequent code concerns the temporary top priorities buffer. But moving todo-show inside save-excursion does prevent point from being relocated, so I made this change as well in the patch. 2009-10-28 Stephen Berman <stephen.berman@gmx.net> * calendar/todo-mode.el (todo-edit-item): Signal an error if there is no item to edit. (Bug#XXXX) (todo-top-priorities): Restore point and restore narrowing in Todo buffer. (Bug#XXXX) [-- Attachment #2: todo-mode.el patch --] [-- Type: text/plain, Size: 4322 bytes --] *** emacs/lisp/calendar/todo-mode.el.~1.76.~ 2009-10-28 11:07:53.000000000 +0100 --- emacs/lisp/calendar/todo-mode.el 2009-10-28 12:55:58.000000000 +0100 *************** *** 505,518 **** (defun todo-edit-item () "Edit current TODO list entry." (interactive) ! (let ((item (todo-item-string))) ! (if (todo-string-multiline-p item) ! (todo-edit-multiline) ! (let ((new (read-from-minibuffer "Edit: " item))) ! (todo-remove-item) ! (insert new "\n") ! (todo-backward-item) ! (message ""))))) (defalias 'todo-cmd-edit 'todo-edit-item) (defun todo-edit-multiline () --- 505,520 ---- (defun todo-edit-item () "Edit current TODO list entry." (interactive) ! (if (< (point-min) (point-max)) ! (let ((item (todo-item-string))) ! (if (todo-string-multiline-p item) ! (todo-edit-multiline) ! (let ((new (read-from-minibuffer "Edit: " item))) ! (todo-remove-item) ! (insert new "\n") ! (todo-backward-item) ! (message "")))) ! (error "No TODO list entry to edit"))) (defalias 'todo-cmd-edit 'todo-edit-item) (defun todo-edit-multiline () *************** *** 745,778 **** (regexp-quote todo-prefix) " " todo-category-sep "\n") (concat todo-category-end "\n")))) beg end) - (todo-show) (save-excursion (save-restriction ! (widen) ! (copy-to-buffer todo-print-buffer-name (point-min) (point-max)) ! (set-buffer todo-print-buffer-name) ! (goto-char (point-min)) ! (when (re-search-forward (regexp-quote todo-header) nil t) ! (beginning-of-line 1) ! (delete-region (point) (line-end-position))) ! (while (re-search-forward ;Find category start ! (regexp-quote (concat todo-prefix todo-category-beg)) ! nil t) ! (setq beg (+ (line-end-position) 1)) ;Start of first entry. ! (re-search-forward cat-end nil t) ! (setq end (match-beginning 0)) ! (replace-match todo-category-break) ! (narrow-to-region beg end) ;In case we have too few entries. ! (goto-char (point-min)) ! (if (zerop nof-priorities) ;Traverse entries. ! (goto-char end) ;All entries ! (todo-forward-item nof-priorities)) ! (setq beg (point)) ! (delete-region beg end) ! (widen)) ! (and (looking-at "\f") (replace-match "")) ;Remove trailing form-feed. ! (goto-char (point-min)) ;Due to display buffer ! )) ;; Could have used switch-to-buffer as it has a norecord argument, ;; which is nice when we are called from e.g. todo-print. ;; Else we could have used pop-to-buffer. --- 747,781 ---- (regexp-quote todo-prefix) " " todo-category-sep "\n") (concat todo-category-end "\n")))) beg end) (save-excursion + (todo-show) (save-restriction ! (save-current-buffer ! (widen) ! (copy-to-buffer todo-print-buffer-name (point-min) (point-max)) ! (set-buffer todo-print-buffer-name) ! (goto-char (point-min)) ! (when (re-search-forward (regexp-quote todo-header) nil t) ! (beginning-of-line 1) ! (delete-region (point) (line-end-position))) ! (while (re-search-forward ;Find category start ! (regexp-quote (concat todo-prefix todo-category-beg)) ! nil t) ! (setq beg (+ (line-end-position) 1)) ;Start of first entry. ! (re-search-forward cat-end nil t) ! (setq end (match-beginning 0)) ! (replace-match todo-category-break) ! (narrow-to-region beg end) ;In case we have too few entries. ! (goto-char (point-min)) ! (if (zerop nof-priorities) ;Traverse entries. ! (goto-char end) ;All entries ! (todo-forward-item nof-priorities)) ! (setq beg (point)) ! (delete-region beg end) ! (widen)) ! (and (looking-at "\f") (replace-match "")) ;Remove trailing form-feed. ! (goto-char (point-min)) ;Due to display buffer ! ))) ;; Could have used switch-to-buffer as it has a norecord argument, ;; which is nice when we are called from e.g. todo-print. ;; Else we could have used pop-to-buffer. ^ permalink raw reply [flat|nested] 3+ messages in thread
* bug#4820: marked as done (23.1.50; [PATCH] todo-mode.el) 2009-10-28 12:20 ` bug#4820: 23.1.50; [PATCH] todo-mode.el Stephen Berman @ 2009-10-31 2:30 ` Emacs bug Tracking System 2009-11-03 22:26 ` bug#4820: 23.1.50; [PATCH] todo-mode.el Stefan Monnier 1 sibling, 0 replies; 3+ messages in thread From: Emacs bug Tracking System @ 2009-10-31 2:30 UTC (permalink / raw) To: Glenn Morris [-- Attachment #1: Type: text/plain, Size: 858 bytes --] Your message dated Fri, 30 Oct 2009 22:23:29 -0400 with message-id <k5hbtg9u8u.fsf@fencepost.gnu.org> and subject line Re: bug#4820: 23.1.50; [PATCH] todo-mode.el has caused the Emacs bug report #4820, regarding 23.1.50; [PATCH] todo-mode.el to be marked as done. This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact owner@emacsbugs.donarmstrong.com immediately.) -- 4820: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=4820 Emacs Bug Tracking System Contact owner@emacsbugs.donarmstrong.com with problems [-- Attachment #2: Type: message/rfc822, Size: 9548 bytes --] [-- Attachment #2.1.1: Type: text/plain, Size: 2909 bytes --] Here are two bugs in todo-mode.el; a patch is attached. 1. emacs -Q with no existing file as the value of todo-file-do 2. M-x todo-show 3. Type `e' (todo-edit-item) => Args out of range: 60, 61 This is because there is no todo item, so when todo-edit-item calls todo-item-string, todo-item-start returns (point-min), which is 61, and todo-item-end returns (1- (line-beginning-position)), i.e. 60, which raises the args-out-of-range error on buffer-substring in todo-item-string. The attached fix to todo-edit-item checks whether there is a todo item to edit and throws an error if not. This is consistent with other Todo mode commands, e.g. todo-{raise, lower, delete, file}-item. 1. emacs -Q 2. M-x todo-show 3. Type `i' and at the prompt a todo item, e.g "test", then RET to display the item in the TODO buffer in the category "Todo"; note that narrowing is in effect, as required by Todo mode. 4. Type `t' to display the Todo top priorities list in a window below the window displaying the TODO buffer. 5. In the TODO buffer type `E' on the item to put it in Todo Edit mode in the buffer *TODO Edit*. 6. Type C-x k to kill *TODO Edit* and return to the TODO buffer, which remains unchanged. 7. In the TODO buffer type `t' again. => Now narrowing is no longer in effect in the TODO buffer. This is because todo-top-priorities, although it calls widen within save-restriction, subsequently calls set-buffer, with the result, after the call to make-indirect-buffer in todo-edit-multiline (step 5 above), that narrowing is not restored. The causal chain here is not quite clear to me: in (elisp)Current Buffer there is a warning to use set-buffer within save-current-buffer or save-excursion to guarantee restoration, and in fact in todo-top-priorities set-buffer is within save-excursion, but that is outside of the save-restriction. If the order of save-excursion and save-restriction is switched, then narrowing is correctly restored after step 7 above; however, this order is discouraged in (elisp)Narrowing. But putting save-current-buffer between save-restriction and set-buffer also restores narrowing, so this is what the attached patch does. (I don't know why the failure to restore narrowing is conditioned by make-indirect-buffer; I asked about this on emacs-devel but have not yet gotten any response.) In addition, the use of save-excursion appears to be gratuitous here: point is already moved by the preceding call to todo-show, and subsequent code concerns the temporary top priorities buffer. But moving todo-show inside save-excursion does prevent point from being relocated, so I made this change as well in the patch. 2009-10-28 Stephen Berman <stephen.berman@gmx.net> * calendar/todo-mode.el (todo-edit-item): Signal an error if there is no item to edit. (Bug#XXXX) (todo-top-priorities): Restore point and restore narrowing in Todo buffer. (Bug#XXXX) [-- Attachment #2.1.2: todo-mode.el patch --] [-- Type: text/plain, Size: 4322 bytes --] *** emacs/lisp/calendar/todo-mode.el.~1.76.~ 2009-10-28 11:07:53.000000000 +0100 --- emacs/lisp/calendar/todo-mode.el 2009-10-28 12:55:58.000000000 +0100 *************** *** 505,518 **** (defun todo-edit-item () "Edit current TODO list entry." (interactive) ! (let ((item (todo-item-string))) ! (if (todo-string-multiline-p item) ! (todo-edit-multiline) ! (let ((new (read-from-minibuffer "Edit: " item))) ! (todo-remove-item) ! (insert new "\n") ! (todo-backward-item) ! (message ""))))) (defalias 'todo-cmd-edit 'todo-edit-item) (defun todo-edit-multiline () --- 505,520 ---- (defun todo-edit-item () "Edit current TODO list entry." (interactive) ! (if (< (point-min) (point-max)) ! (let ((item (todo-item-string))) ! (if (todo-string-multiline-p item) ! (todo-edit-multiline) ! (let ((new (read-from-minibuffer "Edit: " item))) ! (todo-remove-item) ! (insert new "\n") ! (todo-backward-item) ! (message "")))) ! (error "No TODO list entry to edit"))) (defalias 'todo-cmd-edit 'todo-edit-item) (defun todo-edit-multiline () *************** *** 745,778 **** (regexp-quote todo-prefix) " " todo-category-sep "\n") (concat todo-category-end "\n")))) beg end) - (todo-show) (save-excursion (save-restriction ! (widen) ! (copy-to-buffer todo-print-buffer-name (point-min) (point-max)) ! (set-buffer todo-print-buffer-name) ! (goto-char (point-min)) ! (when (re-search-forward (regexp-quote todo-header) nil t) ! (beginning-of-line 1) ! (delete-region (point) (line-end-position))) ! (while (re-search-forward ;Find category start ! (regexp-quote (concat todo-prefix todo-category-beg)) ! nil t) ! (setq beg (+ (line-end-position) 1)) ;Start of first entry. ! (re-search-forward cat-end nil t) ! (setq end (match-beginning 0)) ! (replace-match todo-category-break) ! (narrow-to-region beg end) ;In case we have too few entries. ! (goto-char (point-min)) ! (if (zerop nof-priorities) ;Traverse entries. ! (goto-char end) ;All entries ! (todo-forward-item nof-priorities)) ! (setq beg (point)) ! (delete-region beg end) ! (widen)) ! (and (looking-at "\f") (replace-match "")) ;Remove trailing form-feed. ! (goto-char (point-min)) ;Due to display buffer ! )) ;; Could have used switch-to-buffer as it has a norecord argument, ;; which is nice when we are called from e.g. todo-print. ;; Else we could have used pop-to-buffer. --- 747,781 ---- (regexp-quote todo-prefix) " " todo-category-sep "\n") (concat todo-category-end "\n")))) beg end) (save-excursion + (todo-show) (save-restriction ! (save-current-buffer ! (widen) ! (copy-to-buffer todo-print-buffer-name (point-min) (point-max)) ! (set-buffer todo-print-buffer-name) ! (goto-char (point-min)) ! (when (re-search-forward (regexp-quote todo-header) nil t) ! (beginning-of-line 1) ! (delete-region (point) (line-end-position))) ! (while (re-search-forward ;Find category start ! (regexp-quote (concat todo-prefix todo-category-beg)) ! nil t) ! (setq beg (+ (line-end-position) 1)) ;Start of first entry. ! (re-search-forward cat-end nil t) ! (setq end (match-beginning 0)) ! (replace-match todo-category-break) ! (narrow-to-region beg end) ;In case we have too few entries. ! (goto-char (point-min)) ! (if (zerop nof-priorities) ;Traverse entries. ! (goto-char end) ;All entries ! (todo-forward-item nof-priorities)) ! (setq beg (point)) ! (delete-region beg end) ! (widen)) ! (and (looking-at "\f") (replace-match "")) ;Remove trailing form-feed. ! (goto-char (point-min)) ;Due to display buffer ! ))) ;; Could have used switch-to-buffer as it has a norecord argument, ;; which is nice when we are called from e.g. todo-print. ;; Else we could have used pop-to-buffer. [-- Attachment #3: Type: message/rfc822, Size: 1554 bytes --] From: Glenn Morris <rgm@gnu.org> To: 4820-done@emacsbugs.donarmstrong.com Subject: Re: bug#4820: 23.1.50; [PATCH] todo-mode.el Date: Fri, 30 Oct 2009 22:23:29 -0400 Message-ID: <k5hbtg9u8u.fsf@fencepost.gnu.org> Thanks; applied. ^ permalink raw reply [flat|nested] 3+ messages in thread
* bug#4820: 23.1.50; [PATCH] todo-mode.el 2009-10-28 12:20 ` bug#4820: 23.1.50; [PATCH] todo-mode.el Stephen Berman 2009-10-31 2:30 ` bug#4820: marked as done (23.1.50; [PATCH] todo-mode.el) Emacs bug Tracking System @ 2009-11-03 22:26 ` Stefan Monnier 1 sibling, 0 replies; 3+ messages in thread From: Stefan Monnier @ 2009-11-03 22:26 UTC (permalink / raw) To: Stephen Berman; +Cc: 4820 > 1. emacs -Q with no existing file as the value of todo-file-do > 2. M-x todo-show > 3. Type `e' (todo-edit-item) > => Args out of range: 60, 61 I've installed a different fix, which may not result in a clean error message, but was needed for other reasons anyway. > 1. emacs -Q > 2. M-x todo-show > 3. Type `i' and at the prompt a todo item, e.g "test", then RET to > display the item in the TODO buffer in the category "Todo"; note that > narrowing is in effect, as required by Todo mode. > 4. Type `t' to display the Todo top priorities list in a window below > the window displaying the TODO buffer. > 5. In the TODO buffer type `E' on the item to put it in Todo Edit mode > in the buffer *TODO Edit*. > 6. Type C-x k to kill *TODO Edit* and return to the TODO buffer, which > remains unchanged. > 7. In the TODO buffer type `t' again. > => Now narrowing is no longer in effect in the TODO buffer. Thank you. I believe the fix to save-restriction I just installed should make this workaround unnecessary. Stefan ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-11-03 22:26 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <k5hbtg9u8u.fsf@fencepost.gnu.org> 2009-10-28 12:20 ` bug#4820: 23.1.50; [PATCH] todo-mode.el Stephen Berman 2009-10-31 2:30 ` bug#4820: marked as done (23.1.50; [PATCH] todo-mode.el) Emacs bug Tracking System 2009-11-03 22:26 ` bug#4820: 23.1.50; [PATCH] todo-mode.el Stefan Monnier
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.