Re: [PATCH] ol-man.el (org-man-open): Set window point not buffer point
Tom Gillespie writes: >> (while (process-live-p process) >> (accept-process-output process))) > > When I tried this before it didn't work, but now it does, I > must have missed something. Patch updated accordingly. > > The order in which the man.el code does things is supremely > confusing, but I think when accept-process-output returns that > means the process sentinel has finished its final run and the > man buffer is fully populated so it is safe to search. Thanks! Applied onto main via 76643256f. https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=76643256f2bddb79908a41c264c43de3e06a70dd -- Ihor Radchenko, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92
Re: [PATCH] ol-man.el (org-man-open): Set window point not buffer point
> (while (process-live-p process) > (accept-process-output process))) When I tried this before it didn't work, but now it does, I must have missed something. Patch updated accordingly. The order in which the man.el code does things is supremely confusing, but I think when accept-process-output returns that means the process sentinel has finished its final run and the man buffer is fully populated so it is safe to search. > Also, compiling the patch yields No byte compiler errors now, and I think I got all the formatting issues. From 848d6fc9bd395d7d45f14af71c4df8ea44ed7b4c Mon Sep 17 00:00:00 2001 From: Tom Gillespie Date: Thu, 28 Jul 2022 23:33:22 -0700 Subject: [PATCH] ol-man: Set window point not buffer point and wait before search * lisp/ol-man.el (org-man-open): Set window point not buffer point and wait before search. When passed man:path::SEARCH `org-man-open' uses `search-forward' to jump to the location of e.g. a heading. Prior to this fix it only used `search-forward', which will not change the point of the cursor in the window, meaning that even if there is a match it will not appear. Use `accept-process-output' to block until the manpage finishes rendering before searching the buffer so that there will be something to find. --- lisp/ol-man.el | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/lisp/ol-man.el b/lisp/ol-man.el index aa22964c5..24e896f30 100644 --- a/lisp/ol-man.el +++ b/lisp/ol-man.el @@ -43,12 +43,22 @@ If PATH contains extra ::STRING which will use `occur' to search matched strings in man buffer." (string-match "\\(.*?\\)\\(?:::\\(.*\\)\\)?$" path) (let* ((command (match-string 1 path)) - (search (match-string 2 path))) -(funcall org-man-command command) + (search (match-string 2 path)) + (buffer (funcall org-man-command command))) (when search - (with-current-buffer (concat "*Man " command "*") - (goto-char (point-min)) - (search-forward search) + (with-current-buffer buffer +(goto-char (point-min)) +(unless (search-forward search nil t) + (let ((process (get-buffer-process buffer))) +(while (process-live-p process) + (accept-process-output process))) + (goto-char (point-min)) + (search-forward search)) +(forward-line -1) +(let ((point (point))) + (let ((window (get-buffer-window buffer))) +(set-window-point window point) +(set-window-start window point))) (defun org-man-store-link () "Store a link to a README file." -- 2.35.1
Re: [PATCH] ol-man.el (org-man-open): Set window point not buffer point
Tom Gillespie writes: > Hi Ihor, >Here is an updated patch. We can't use accept-process-output > because it doesn't seem to block in the way we need, or it blocks > exactly long enough for the process to finish but then continues > immediately to search instead of allowing the function that fills > the buffer to complete. Instead I use sleep-for a shorter time and > process-live-p which gives better results. I think I got the commit > message formats right this time. Best! Why not (while (process-live-p process) (accept-process-output process))) then? sleep-for is using similar machinery under the hood, but accept-process-output does not require magic constants and in addition handles various edge cases. Also, compiling the patch yields In org-man-open: ol-man.el:54:16: Warning: ‘previous-line’ is for interactive use only; use ‘forward-line’ with negative argument instead. > * lisp/ol-man.el (org-man-open): Set window point not buffer point > When passed man:path::SEARCH org-man-open tries to use search-forward > to jump to the location of e.g. a heading. Prior to this fix it only > used search-forward, which will not change the point of the cursor in > the window, meaning that even if there is a match it will not appear. > Uses process-live-p and sleep-for to wait until the manpage finishes > rendering before searching the buffer so that there will be something > to find. Please use double space " " between sentences and quote `org-man-open' and similar Elisp symbols. -- Ihor Radchenko, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92
Re: [PATCH] ol-man.el (org-man-open): Set window point not buffer point
Hi Ihor, Here is an updated patch. We can't use accept-process-output because it doesn't seem to block in the way we need, or it blocks exactly long enough for the process to finish but then continues immediately to search instead of allowing the function that fills the buffer to complete. Instead I use sleep-for a shorter time and process-live-p which gives better results. I think I got the commit message formats right this time. Best! Tom From 2db2ce6d83b27fcf6366183cbd8b5fa79fcbc4a7 Mon Sep 17 00:00:00 2001 From: Tom Gillespie Date: Thu, 28 Jul 2022 23:33:22 -0700 Subject: [PATCH] ol-man: Set window point not buffer point and wait before search * lisp/ol-man.el (org-man-open): Set window point not buffer point When passed man:path::SEARCH org-man-open tries to use search-forward to jump to the location of e.g. a heading. Prior to this fix it only used search-forward, which will not change the point of the cursor in the window, meaning that even if there is a match it will not appear. Uses process-live-p and sleep-for to wait until the manpage finishes rendering before searching the buffer so that there will be something to find. --- lisp/ol-man.el | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/lisp/ol-man.el b/lisp/ol-man.el index aa22964c5..8633fe5cb 100644 --- a/lisp/ol-man.el +++ b/lisp/ol-man.el @@ -43,12 +43,22 @@ If PATH contains extra ::STRING which will use `occur' to search matched strings in man buffer." (string-match "\\(.*?\\)\\(?:::\\(.*\\)\\)?$" path) (let* ((command (match-string 1 path)) - (search (match-string 2 path))) -(funcall org-man-command command) + (search (match-string 2 path)) + (buffer (funcall org-man-command command))) (when search - (with-current-buffer (concat "*Man " command "*") - (goto-char (point-min)) - (search-forward search) + (with-current-buffer buffer +(goto-char (point-min)) +(unless (search-forward search nil t) + (let ((process (get-buffer-process buffer))) +(while (process-live-p process) + (sleep-for 0.01))) + (goto-char (point-min)) + (search-forward search)) +(previous-line) +(let ((point (point))) + (let ((window (get-buffer-window buffer))) +(set-window-point window point) +(set-window-start window point))) (defun org-man-store-link () "Store a link to a README file." -- 2.35.1
Re: [PATCH] ol-man.el (org-man-open): Set window point not buffer point
Tom Gillespie writes: > Here's a patch to fix the follow behavior for ol-man links so > that the ::SEARCH functionality will actually work. Best! > Tom > From 2c3e3b994fd7b47a6e91d147d2b1f08cd97a1908 Mon Sep 17 00:00:00 2001 > From: Tom Gillespie > Date: Thu, 28 Jul 2022 23:33:22 -0700 > Subject: [PATCH] * lisp/ol-man.el (org-man-open): Set window point not buffer > point Thanks for the contribution! > When passed man:path::SEARCH org-man-open tries to use search-forward > to jump to the location of e.g. a heading. Prior to this fix it only > used search-forward, which will not change the point of the cursor in > the window, meaning that even if there is a match it will not appear. Please check out https://orgmode.org/worg/org-contribute.html#commit-messages for our conventions about commit messages. > Use sleep-for as a horrible hack to work around the fact that the man > command runs in the background with no way to synchronize back. This is indeed a horrible hack. What about `accept-process-output'? Best, Ihor
[PATCH] ol-man.el (org-man-open): Set window point not buffer point
Here's a patch to fix the follow behavior for ol-man links so that the ::SEARCH functionality will actually work. Best! Tom From 2c3e3b994fd7b47a6e91d147d2b1f08cd97a1908 Mon Sep 17 00:00:00 2001 From: Tom Gillespie Date: Thu, 28 Jul 2022 23:33:22 -0700 Subject: [PATCH] * lisp/ol-man.el (org-man-open): Set window point not buffer point When passed man:path::SEARCH org-man-open tries to use search-forward to jump to the location of e.g. a heading. Prior to this fix it only used search-forward, which will not change the point of the cursor in the window, meaning that even if there is a match it will not appear. Use sleep-for as a horrible hack to work around the fact that the man command runs in the background with no way to synchronize back. --- lisp/ol-man.el | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lisp/ol-man.el b/lisp/ol-man.el index aa22964c5..5843cd5f6 100644 --- a/lisp/ol-man.el +++ b/lisp/ol-man.el @@ -43,12 +43,20 @@ If PATH contains extra ::STRING which will use `occur' to search matched strings in man buffer." (string-match "\\(.*?\\)\\(?:::\\(.*\\)\\)?$" path) (let* ((command (match-string 1 path)) - (search (match-string 2 path))) -(funcall org-man-command command) + (search (match-string 2 path)) + (buffer (funcall org-man-command command))) (when search - (with-current-buffer (concat "*Man " command "*") - (goto-char (point-min)) - (search-forward search) + (with-current-buffer buffer +(goto-char (point-min)) +(unless (search-forward search nil t) + (sleep-for 0.75) ; async, can't block, no callback + (goto-char (point-min)) + (search-forward search)) +(previous-line) +(let ((point (point))) + (let ((window (get-buffer-window buffer))) +(set-window-point window point) +(set-window-start window point))) (defun org-man-store-link () "Store a link to a README file." -- 2.35.1
