* [PATCH] ol-man.el (org-man-open): Set window point not buffer point
@ 2022-07-29 6:47 Tom Gillespie
2022-07-29 13:20 ` Ihor Radchenko
0 siblings, 1 reply; 6+ messages in thread
From: Tom Gillespie @ 2022-07-29 6:47 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 124 bytes --]
Here's a patch to fix the follow behavior for ol-man links so
that the ::SEARCH functionality will actually work. Best!
Tom
[-- Attachment #2: 0001-lisp-ol-man.el-org-man-open-Set-window-point-not-buf.patch --]
[-- Type: text/x-patch, Size: 1914 bytes --]
From 2c3e3b994fd7b47a6e91d147d2b1f08cd97a1908 Mon Sep 17 00:00:00 2001
From: Tom Gillespie <tgbugs@gmail.com>
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ol-man.el (org-man-open): Set window point not buffer point
2022-07-29 6:47 [PATCH] ol-man.el (org-man-open): Set window point not buffer point Tom Gillespie
@ 2022-07-29 13:20 ` Ihor Radchenko
2022-08-09 1:39 ` Tom Gillespie
0 siblings, 1 reply; 6+ messages in thread
From: Ihor Radchenko @ 2022-07-29 13:20 UTC (permalink / raw)
To: Tom Gillespie; +Cc: emacs-orgmode
Tom Gillespie <tgbugs@gmail.com> 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 <tgbugs@gmail.com>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ol-man.el (org-man-open): Set window point not buffer point
2022-07-29 13:20 ` Ihor Radchenko
@ 2022-08-09 1:39 ` Tom Gillespie
2022-08-09 12:13 ` Ihor Radchenko
0 siblings, 1 reply; 6+ messages in thread
From: Tom Gillespie @ 2022-08-09 1:39 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 448 bytes --]
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
[-- Attachment #2: 0001-ol-man-Set-window-point-not-buffer-point-and-wait-be.patch --]
[-- Type: text/x-patch, Size: 2067 bytes --]
From 2db2ce6d83b27fcf6366183cbd8b5fa79fcbc4a7 Mon Sep 17 00:00:00 2001
From: Tom Gillespie <tgbugs@gmail.com>
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ol-man.el (org-man-open): Set window point not buffer point
2022-08-09 1:39 ` Tom Gillespie
@ 2022-08-09 12:13 ` Ihor Radchenko
2022-08-10 0:41 ` Tom Gillespie
0 siblings, 1 reply; 6+ messages in thread
From: Ihor Radchenko @ 2022-08-09 12:13 UTC (permalink / raw)
To: Tom Gillespie; +Cc: emacs-orgmode
Tom Gillespie <tgbugs@gmail.com> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ol-man.el (org-man-open): Set window point not buffer point
2022-08-09 12:13 ` Ihor Radchenko
@ 2022-08-10 0:41 ` Tom Gillespie
2022-08-10 11:47 ` Ihor Radchenko
0 siblings, 1 reply; 6+ messages in thread
From: Tom Gillespie @ 2022-08-10 0:41 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 540 bytes --]
> (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.
[-- Attachment #2: 0001-ol-man-Set-window-point-not-buffer-point-and-wait-be.patch --]
[-- Type: text/x-patch, Size: 2104 bytes --]
From 848d6fc9bd395d7d45f14af71c4df8ea44ed7b4c Mon Sep 17 00:00:00 2001
From: Tom Gillespie <tgbugs@gmail.com>
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ol-man.el (org-man-open): Set window point not buffer point
2022-08-10 0:41 ` Tom Gillespie
@ 2022-08-10 11:47 ` Ihor Radchenko
0 siblings, 0 replies; 6+ messages in thread
From: Ihor Radchenko @ 2022-08-10 11:47 UTC (permalink / raw)
To: Tom Gillespie; +Cc: emacs-orgmode
Tom Gillespie <tgbugs@gmail.com> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-10 11:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-29 6:47 [PATCH] ol-man.el (org-man-open): Set window point not buffer point Tom Gillespie
2022-07-29 13:20 ` Ihor Radchenko
2022-08-09 1:39 ` Tom Gillespie
2022-08-09 12:13 ` Ihor Radchenko
2022-08-10 0:41 ` Tom Gillespie
2022-08-10 11:47 ` Ihor Radchenko
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).