emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Lazy load of org-protocol
@ 2022-02-05 11:54 Max Nikulin
  2022-02-05 18:27 ` Jim Porter
  0 siblings, 1 reply; 15+ messages in thread
From: Max Nikulin @ 2022-02-05 11:54 UTC (permalink / raw)
  To: emacs-orgmode

Hi.

I would prefer to avoid
    (require 'org-protocol)
in emacs init file and to postpone loading till invocation of 
emacsclient with org-protocol URI.

The problem is a hack in org-protocol. URIs are actually treated as 
(relative) file names and magic is achieved in an advice for 
`server-visit-files' function. So the advice must be installed in advance.

My first idea was to avoid such magic and to create autoload function 
org-protocol-from-argv with body similar to that advice. If it were 
possible to get arguments from `command-line-args-left' then invocation 
would look like

    emacsclient --eval '(org-protocol-from-argv)' 
'org-protocol:/store-link?url=u1&title=t1'

Unfortunately it is against design of emacs server protocol. If --eval 
option is given than everything other is considered as independent 
expressions. At the lower level there is no way to transfer `argv` as a 
list from client to server process.

It seems, it works if I add another advice to init.el that loads 
org-protocol on demand:

(defadvice server-visit-files (before org-protocol-lazy-load activate)
   (and (not (featurep 'org-protocol))
        (memq nil
	     (mapcar (lambda (loc)
		       ;; loc: (file-name . (line . column))
		       (not (string-match-p "\\(?:^\\|[/\\\\]\\)org-protocol:" (car 
loc))))
		     (ad-get-arg 0)))
        (progn
	 (require 'org-protocol)
	 ;; copy of org-protocol-detect-protocol-server advice,
          ;; move to a dedicated function
	 (let ((flist (if org-protocol-reverse-list-of-files
			  (reverse  (ad-get-arg 0))
			(ad-get-arg 0)))
	       (client (ad-get-arg 1)))
	   (catch 'greedy
	     (dolist (var flist)
	       ;; `\' to `/' on windows.  FIXME: could this be done any better?
	       (let ((fname  (expand-file-name (car var))))
		 (setq fname (org-protocol-check-filename-for-protocol
			      fname (member var flist)  client))
		 (if (eq fname t) ;; greedy? We need the t return value.
		     (progn
		       (ad-set-arg 0 nil)
		       (throw 'greedy t))
		   (if (stringp fname) ;; probably filename
		       (setcar var fname)
		     (ad-set-arg 0 (delq var (ad-get-arg 0))))))))))))

I hope, copy of original advice may be avoided by moving its body to a 
separate function in org-protocol.

Do you have have better ideas how to avoid eager loading of org-protocol 
from init file?



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Lazy load of org-protocol
  2022-02-05 11:54 Lazy load of org-protocol Max Nikulin
@ 2022-02-05 18:27 ` Jim Porter
  2022-02-06 16:42   ` Max Nikulin
  0 siblings, 1 reply; 15+ messages in thread
From: Jim Porter @ 2022-02-05 18:27 UTC (permalink / raw)
  To: Max Nikulin, emacs-orgmode

On 2/5/2022 3:54 AM, Max Nikulin wrote:
> I would prefer to avoid
>     (require 'org-protocol)
> in emacs init file and to postpone loading till invocation of 
> emacsclient with org-protocol URI.
> 
> The problem is a hack in org-protocol. URIs are actually treated as 
> (relative) file names and magic is achieved in an advice for 
> `server-visit-files' function. So the advice must be installed in advance.
> 
> My first idea was to avoid such magic and to create autoload function 
> org-protocol-from-argv with body similar to that advice. If it were 
> possible to get arguments from `command-line-args-left' then invocation 
> would look like
> 
>     emacsclient --eval '(org-protocol-from-argv)' 
> 'org-protocol:/store-link?url=u1&title=t1'
> 
> Unfortunately it is against design of emacs server protocol. If --eval 
> option is given than everything other is considered as independent 
> expressions. At the lower level there is no way to transfer `argv` as a 
> list from client to server process.

I've thought a bit about how to improve this too. One further issue with 
the current implementation is that when emacsclient invokes the 
alternate editor (usually to start the "main" emacs instance), 
org-protocol: links no longer work correctly. That's because only the 
emacsclient itself (through `server-visit-files') knows what to do with 
them.

I think the problem really starts in emacsclient's command line 
handling. You can see this in other situations too. For example, Emacs 
can be configured as your system's mailto: URL handler. (The file 
etc/emacsclient-mail.desktop in the Emacs repo does this.) The command 
to use for a new Emacs instance is simple:

   emacs -f message-mailto %u

However, doing this for emacsclient is harder:

   emacsclient --alternate-editor= --create-frame --eval 
"(message-mailto \\"%u\\")"

There's no problem with "--alternate-editor=" and "--create-frame", but 
the fact that emacsclient requires evaling the function call that way 
is: if %u holds a string with quotation marks, this will break, and 
worse, could even result in arbitrary code being executed. (In practice, 
this is probably rare, since URLs are generally URL-encoded, and so 
don't have literal quotes in them.)

As a result, I think a good first step might be to add support for 
"--funcall" to emacsclient, just like the regular emacs binary. (The 
"-f" shorthand won't work though, since emacsclient already uses that 
for "--server-file"). This would simplify the `message-mailto' case 
above and would also allow org-protocol to do something similar:

   emacsclient --funcall org-protocol-capture %u

Then, so long as `org-protocol-capture' has an autoload, this would Just 
Work, and org-protocol.el would be lazily loaded. Hopefully, this could 
also be forwarded onto the alternate editor so that when the user open 
an org-protocol: URL when Emacs is closed, it still handles the URL 
correctly.

That said, in the short term, you could try out something like:

   emacsclient --eval "(org-protocol-capture \\"%u\\")"

Of course, that has the quoting issues I mentioned above, but it could 
be helpful for developing a proof of concept.

- Jim


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Lazy load of org-protocol
  2022-02-05 18:27 ` Jim Porter
@ 2022-02-06 16:42   ` Max Nikulin
  2022-02-06 19:40     ` Jim Porter
  0 siblings, 1 reply; 15+ messages in thread
From: Max Nikulin @ 2022-02-06 16:42 UTC (permalink / raw)
  To: emacs-orgmode

On 06/02/2022 01:27, Jim Porter wrote:
> On 2/5/2022 3:54 AM, Max Nikulin wrote:
> etc/emacsclient-mail.desktop in the Emacs repo does this.) The command 
> to use for a new Emacs instance is simple:
> 
>    emacs -f message-mailto %u
> 
> However, doing this for emacsclient is harder:
> 
>    emacsclient --alternate-editor= --create-frame --eval 
> "(message-mailto \\"%u\\")"
> 
> There's no problem with "--alternate-editor=" and "--create-frame", but 
> the fact that emacsclient requires evaling the function call that way 
> is: if %u holds a string with quotation marks, this will break, and 
> worse, could even result in arbitrary code being executed. (In practice, 
> this is probably rare, since URLs are generally URL-encoded, and so 
> don't have literal quotes in them.)

Thank you for suggesting another use case.

Quoting issues was the reason why I started to search a better way. 
There should be an easy and safe means to pass argument from command 
line to evaluated expressions similar to shell
     sh -c 'echo "$1"' example 'Hello, World!'

Some people could not even choose proper quotes for shell command:
https://www.reddit.com/r/emacs/comments/hhbcg7/emacsclient_eval_with_command_line_arguments/
https://stackoverflow.com/questions/8848819/emacs-eval-ediff-1-2-how-to-put-this-line-in-to-shell-script
First recipe and the accepted answer in second source solves the obvious 
problem but they miss escaping for elisp expression. Another answer on 
stackoverflow is more accurate, it suggests
     quoted1=${1//\\/\\\\}; quoted1=${quoted1//\"/\\\"}
I suppose, these links is a good illustration that substitution of 
arbitrary argument into lisp expression is harder than it should be to 
help users to avoid security issues.

> As a result, I think a good first step might be to add support for 
> "--funcall" to emacsclient, just like the regular emacs binary. (The 
> "-f" shorthand won't work though, since emacsclient already uses that 
> for "--server-file"). This would simplify the `message-mailto' case 
> above and would also allow org-protocol to do something similar:
> 
>    emacsclient --funcall org-protocol-capture %u

No, --funcall is just a sugar for --eval '(func)' that does not contain 
arbitrary input, but func has no access to other arguments and it is the 
real problem.

I think, the solution is to add -arg command to emacs server protocol 
that pushes its argument to a list and extend -exec command that would 
make such list available as argv or as `command-line-args-left' for 
evaluated expression. Of course, emacsclient option parser should be 
modified as well to support --arg option
      emacsclient --eval '(func)' --arg 1 2 3
      emacsclient --eval '(func)' --arg -- 1 2 3
and maybe even for multiple eval+arg pairs
      emacsclient --eval '(f1)' --arg 'a1' --eval '(f2)' --arg 'a2' 'a3'

The proper place to discuss idea is emacs-devel list, but I am afraid 
that without a patch it will be just buried.

>    emacsclient --eval "(org-protocol-capture \\"%u\\")"

Due to quoting issues a small wrapper may be safer (modulo -a, -c)

     emacsclient --eval "(require 'org-protocol)"
     emacsclient -- "$@"



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Lazy load of org-protocol
  2022-02-06 16:42   ` Max Nikulin
@ 2022-02-06 19:40     ` Jim Porter
  2022-02-07 14:57       ` Max Nikulin
  0 siblings, 1 reply; 15+ messages in thread
From: Jim Porter @ 2022-02-06 19:40 UTC (permalink / raw)
  To: emacs-orgmode

On 2/6/2022 8:42 AM, Max Nikulin wrote:
> On 06/02/2022 01:27, Jim Porter wrote:
>> As a result, I think a good first step might be to add support for 
>> "--funcall" to emacsclient, just like the regular emacs binary. (The 
>> "-f" shorthand won't work though, since emacsclient already uses that 
>> for "--server-file"). This would simplify the `message-mailto' case 
>> above and would also allow org-protocol to do something similar:
>>
>>    emacsclient --funcall org-protocol-capture %u
> 
> No, --funcall is just a sugar for --eval '(func)' that does not contain 
> arbitrary input, but func has no access to other arguments and it is the 
> real problem.

Oh right, of course. The emacsclient case with `(message-mailto \"%u\")' 
threw me off, but `message-mailto' works differently depending on 
whether you pass it an arg or not.

> I think, the solution is to add -arg command to emacs server protocol 
> that pushes its argument to a list and extend -exec command that would 
> make such list available as argv or as `command-line-args-left' for 
> evaluated expression. Of course, emacsclient option parser should be 
> modified as well to support --arg option
>       emacsclient --eval '(func)' --arg 1 2 3
>       emacsclient --eval '(func)' --arg -- 1 2 3
> and maybe even for multiple eval+arg pairs
>       emacsclient --eval '(f1)' --arg 'a1' --eval '(f2)' --arg 'a2' 'a3'

I think something like this could work, so long as the arguments can be 
forwarded correctly in the alternate editor case. If I understand how 
emacsclient handles --eval, I think that might work, but it would still 
require writing Lisp functions that know how to handle argv or 
`command-line-args-left'. I'm also not totally sure how safe/sensible 
that is when the arguments aren't from the invocation of emacs itself, 
but from emacsclient (and thus, the args could be updated multiple times 
throughout a session). It probably wouldn't actually break anything, but 
it does seem a bit surprising to me...

Maybe another option would be to add an --apply argument that really 
*does* consume the other command-line args and turns them into a 
properly-quoted function call. Roughly speaking, it would turn this:

   emacs --apply func foo bar baz

into this:

   (apply #'func '(foo bar baz))

This would work effectively like how I momentarily thought --funcall 
works. Then you could say:

   emacsclient --apply org-protocol-capture %u
   # or ...
   emacs --apply org-protocol-capture %u

This would also mean that the `message-mailto' function from Gnus 
doesn't need to care about `command-line-args-left' anymore, which would 
simplify it somewhat. And this could be useful in general for 
programmatically interacting with Emacs, since users would be able to 
pass arbitrary strings to Emacs Lisp functions without having to worry 
(as much) about sanitizing the strings first.

> The proper place to discuss idea is emacs-devel list, but I am afraid 
> that without a patch it will be just buried.

I don't know if an actual patch would be a strict prerequisite for 
posting to emacs-devel, but it's probably wise to have a clear, detailed 
proposal first. I think it's easier to get everyone on board with an 
idea if it's easy for them to see how all the details work up-front.

- Jim


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Lazy load of org-protocol
  2022-02-06 19:40     ` Jim Porter
@ 2022-02-07 14:57       ` Max Nikulin
  2022-02-07 19:06         ` Jim Porter
  2022-02-08 10:44         ` Emacs-orgmode Digest, Vol 192, Issue 8 Tianshu Wang
  0 siblings, 2 replies; 15+ messages in thread
From: Max Nikulin @ 2022-02-07 14:57 UTC (permalink / raw)
  To: emacs-orgmode

On 06/02/2022 09:40, Tianshu Wang wrote:
> 
> (defadvice server-execute (before enable-org-protocol activate)
>   (unless (featurep 'org-protocol) (require 'org-protocol)))

Thank you, such approach, unlike mine example, does not have code 
duplication. On the other hand it loads org-protocol on any remote 
command, not only for "files" representing org-protocol URIs. Maybe 
defadvice in org-protocol.el should be changed by newer advice-add with 
a function containing body of the old advice.

On 07/02/2022 02:40, Jim Porter wrote:
> On 2/6/2022 8:42 AM, Max Nikulin wrote:
>> On 06/02/2022 01:27, Jim Porter wrote:
> 
>> I think, the solution is to add -arg command to emacs server protocol 
>> that pushes its argument to a list and extend -exec command that would 
>> make such list available as argv or as `command-line-args-left' for 
>> evaluated expression. Of course, emacsclient option parser should be 
>> modified as well to support --arg option
>>       emacsclient --eval '(func)' --arg 1 2 3
>>       emacsclient --eval '(func)' --arg -- 1 2 3
>> and maybe even for multiple eval+arg pairs
>>       emacsclient --eval '(f1)' --arg 'a1' --eval '(f2)' --arg 'a2' 'a3'
> 
> I think something like this could work, so long as the arguments can be 
> forwarded correctly in the alternate editor case. If I understand how 
> emacsclient handles --eval, I think that might work, but it would still 
> require writing Lisp functions that know how to handle argv or 
> `command-line-args-left'. I'm also not totally sure how safe/sensible 
> that is when the arguments aren't from the invocation of emacs itself, 
> but from emacsclient (and thus, the args could be updated multiple times 
> throughout a session). It probably wouldn't actually break anything, but 
> it does seem a bit surprising to me...

Of course, I mean let-bind of `argv' and `command-line-args-left' just 
for processing of the passed expression, not setq that would change them 
globally.

> Maybe another option would be to add an --apply argument that really 
> *does* consume the other command-line args and turns them into a 
> properly-quoted function call. Roughly speaking, it would turn this:
> 
>    emacs --apply func foo bar baz
> 
> into this:
> 
>    (apply #'func '(foo bar baz))

You have almost managed to sell this idea to me. However even --apply is 
just sugar for

     emacsclient --eval "(apply #'func command-line-args-left)" --arg 1 2

So it is --apply option that may require to write a dedicated function 
if --arg is not implemented. Another limitation of --apply is that the 
function must accept string arguments only, no lists or even integers or 
boolean.

> This would work effectively like how I momentarily thought --funcall 
> works. Then you could say:
> 
>    emacsclient --apply org-protocol-capture %u
>    # or ...
>    emacs --apply org-protocol-capture %u

Just because of there were a couple of recent threads related to calling 
conventions for subprotocol handlers, it is better to provide a bit more 
"real" example

     emacsclient --eval '(org-protocol-check-filename-for-protocol (pop 
command-line-args-left) nil nil)' --args %u



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Lazy load of org-protocol
  2022-02-07 14:57       ` Max Nikulin
@ 2022-02-07 19:06         ` Jim Porter
  2022-02-09 16:46           ` Max Nikulin
  2022-02-08 10:44         ` Emacs-orgmode Digest, Vol 192, Issue 8 Tianshu Wang
  1 sibling, 1 reply; 15+ messages in thread
From: Jim Porter @ 2022-02-07 19:06 UTC (permalink / raw)
  To: emacs-orgmode

On 2/7/2022 6:57 AM, Max Nikulin wrote:
>> Maybe another option would be to add an --apply argument that really 
>> *does* consume the other command-line args and turns them into a 
>> properly-quoted function call. Roughly speaking, it would turn this:
>>
>>    emacs --apply func foo bar baz
>>
>> into this:
>>
>>    (apply #'func '(foo bar baz))
> 
> You have almost managed to sell this idea to me. However even --apply is 
> just sugar for
> 
>      emacsclient --eval "(apply #'func command-line-args-left)" --arg 1 2
> 
> So it is --apply option that may require to write a dedicated function 
> if --arg is not implemented. Another limitation of --apply is that the 
> function must accept string arguments only, no lists or even integers or 
> boolean.

Yeah, `--arg' does have greater flexibility in that regard, but it comes 
at the cost of verbosity and some slight behavioral differences with the 
equivalent in the main emacs executable. For emacs itself, any elements 
of `command-line-args-left' that aren't consumed by a function will get 
opened as files, but I don't think that would be the case with 
emacsclient. Does this difference matter? Probably not. However, 
avoiding `command-line-args-left' just feels intuitively "cleaner" to 
me. Still, I think `--arg' should work and be backwards-compatible, so I 
don't mind it, even if I prefer the simplicity of `--apply'.

On a related note, there is still an issue with `--eval' in some cases. 
It fails to work with emacsclient when invoking an alternate editor:

   emacsclient --alternate-editor emacs --eval '(message "hi")'

If Emacs isn't already running, that will open a new buffer named 
'(message "hi")'. I think that's just a bug in how emacsclient handles 
`--eval', though fixing it would make org-protocol links work more 
reliably (if they used `--eval' as proposed, that is).

- Jim


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emacs-orgmode Digest, Vol 192, Issue 8
  2022-02-07 14:57       ` Max Nikulin
  2022-02-07 19:06         ` Jim Porter
@ 2022-02-08 10:44         ` Tianshu Wang
  1 sibling, 0 replies; 15+ messages in thread
From: Tianshu Wang @ 2022-02-08 10:44 UTC (permalink / raw)
  To: emacs-orgmode


Max Nikulin writes:

> Thank you, such approach, unlike mine example, does not have code
> duplication. On the other hand it loads org-protocol on any remote
> command, not only for "files" representing org-protocol URIs. Maybe
> defadvice in org-protocol.el should be changed by newer advice-add with
> a function containing body of the old advice.

Yes, I replaced the original code with advice-add (not fully tested).

(advice-add 'server-execute :before
            (defun enable-org-protocol (&rest r)
              (unless (featurep 'org-protocol) (require 'org-protocol))))

--
Tianshu Wang


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] lisp/org-capture.el: Add hook & hook options to org-capture (Valentin Herrmann)
       [not found] <mailman.61.1644253327.32758.emacs-orgmode@gnu.org>
@ 2022-02-08 19:02 ` No Wayman
  2022-02-09  4:10   ` Ihor Radchenko
  2022-02-10 19:32   ` Greg Minshall
  0 siblings, 2 replies; 15+ messages in thread
From: No Wayman @ 2022-02-08 19:02 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: emacs-orgmode-request


I've implemented what you're proposing here (and much more) in a 
package you may find useful a couple years ago. I pitched adopting 
some of the ideas into org-mode proper and was willing to do the 
work. My proposal was met with enthusiastic silence:

https://www.github.com/progfolio/doct


> - :prepend            Normally newly captured information will 
> be appended at
> -                     the target location (last child, last 
> table line,
> -                     last list item...).  Setting this property 
> will
> -                     change that.
> + :prepend               Normally newly captured information 
> will be appended at
> +                        the target location (last child, last 
> table line,
> +                        last list item...).  Setting this 
> property will
> +                        change that.

Are the white space changes to unrelated properties necessary?
   
> +(defcustom org-capture-before-view-hook nil
> +  "Hook that is run right after the capture buffer is made 
> current.
> +The buffer is still narrowed."
> +  :group 'org-capture
> +  :version "28.1"
> +  :type 'hook)
> +

This functionality is already provided by `org-capture-mode-hook'. 
Instead of introducing another hook, that can be utilized. An 
illustration using doct's :hook keyword which runs functions 
during org-capture-mode-hook:

Yodel[1] Report 2022-02-08 14:16:58
===================================

--8<---------------cut here---------------start------------->8---
(yodel
  :save "org-capture-mode-hook-example"
  :packages*
  doct
  :post*
  (require 'org-capture)
  (require 'doct)
  (let ((org-capture-templates
        (doct
         `("test"
           :keys "t"
           :file "/tmp/test.org"
           :template "* test"
           :immediate-finish t
           :hook (lambda nil
             (message "%S narrowed?: %S"
                      (current-buffer)
                      (buffer-narrowed-p)))))))
    (org-capture nil "t")))
--8<---------------cut here---------------end--------------->8---

STDOUT
======

> Loading 
> /tmp/org-capture-mode-hook-example/straight-bootstrap-snippet.el 
> (source)...
> Clipboard pasted as level 1 subtree
> #<buffer CAPTURE-test.org> narrowed?: t

Environment
===========

- emacs version: GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, 
  GTK+ Version 3.24.31, cairo version 1.17.4)
 of 2022-01-13
- system type: gnu/linux

Packages
========

- doct 
  https://github.com/progfolio/doct/commit/9ed9b8c7f7e2ea2d2fb739d65ae4626a1cf16b9f

[1] https://www.github.com/progfolio/yodel

> +
> +        (run-hooks 'org-capture-before-view-hook)
> +        (if-let ((bvh (org-capture-get :before-view-hook)))
> +            (funcall bvh))

This pattern implies that functions added via template keywords 
will be run after the equivalent global hooks. That should be 
documented. This pattern could be improved by let-binding each 
hook and adding the templates functions. e.g.

>(let ((org-capture-mode-hook
>       (append org-capture-mode-hook (org-capture-get :hook t))))
>  (run-hooks 'org-capture-mode-hook))

Note the use of the non-nil LOCAL argument in org-capture-get.
You want to ensure you're accessing the correct plist in the case 
of overlapping capture processes.
 
>    ;; FIXME: This does not do the right thing, we need to remove 
>    the
>    ;; new stuff by hand it is easy: undo, then kill the buffer
> -  (let ((org-note-abort t)
> -	(org-capture-before-finalize-hook nil))
> +  (let ((org-note-abort t))
>      (org-capture-finalize)))
>  
>  (defun org-capture-goto-last-stored ()

Why was org-capture-before-finalize-hook bound to nil here and 
what is the reason for changing that?




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] lisp/org-capture.el: Add hook & hook options to org-capture (Valentin Herrmann)
  2022-02-08 19:02 ` [PATCH] lisp/org-capture.el: Add hook & hook options to org-capture (Valentin Herrmann) No Wayman
@ 2022-02-09  4:10   ` Ihor Radchenko
  2022-02-09  7:11     ` No Wayman
  2022-02-10 19:32   ` Greg Minshall
  1 sibling, 1 reply; 15+ messages in thread
From: Ihor Radchenko @ 2022-02-09  4:10 UTC (permalink / raw)
  To: No Wayman; +Cc: emacs-orgmode-request, emacs-orgmode

No Wayman <iarchivedmywholelife@gmail.com> writes:

> I've implemented what you're proposing here (and much more) in a 
> package you may find useful a couple years ago. I pitched adopting 
> some of the ideas into org-mode proper and was willing to do the 
> work. My proposal was met with enthusiastic silence:
>
> https://www.github.com/progfolio/doct

I think Nicolas gave some reasonable comments, didn't he? He suggested
to incorporate some of the ideas into the existing Org mode code.

https://orgmode.org/list/87wo66t8i7.fsf@gmail.com

Best,
Ihor


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] lisp/org-capture.el: Add hook & hook options to org-capture (Valentin Herrmann)
  2022-02-09  4:10   ` Ihor Radchenko
@ 2022-02-09  7:11     ` No Wayman
  2022-03-20 10:43       ` Ihor Radchenko
  0 siblings, 1 reply; 15+ messages in thread
From: No Wayman @ 2022-02-09  7:11 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode-request, emacs-orgmode


Ihor Radchenko <yantar92@gmail.com> writes:

> No Wayman <iarchivedmywholelife@gmail.com> writes:
>
>> I've implemented what you're proposing here (and much more) in 
>> a 
>> package you may find useful a couple years ago. I pitched 
>> adopting 
>> some of the ideas into org-mode proper and was willing to do 
>> the 
>> work. My proposal was met with enthusiastic silence:
>>
>> https://www.github.com/progfolio/doct
>
> I think Nicolas gave some reasonable comments, didn't he? He 
> suggested
> to incorporate some of the ideas into the existing Org mode 
> code.
>
> https://orgmode.org/list/87wo66t8i7.fsf@gmail.com
>
> Best,
> Ihor

I believe you're referring to:

https://list.orgmode.org/87y2qlgq33.fsf@nicolasgoaziou.fr/

He had some reasonable questions about the design, but said he 
only uses very basic capture templates, and could not comment 
beyond that on the design. The hope was that people who use more 
of org-capture's features would chime in. That didn't happen.

Of course doct's features could be added to org-capture in a 
piecemeal fashion, but I'm not too keen on contributing anything 
non-trivial to Org these days. Too much back and forth coupled 
with discussion that can stall due to lack of interest. 
e.g.

https://list.orgmode.org/87h7e6dluc.fsf@gmail.com/
https://list.orgmode.org/87tuit73ij.fsf@gmail.com/





^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Lazy load of org-protocol
  2022-02-07 19:06         ` Jim Porter
@ 2022-02-09 16:46           ` Max Nikulin
  2022-02-09 19:22             ` Jim Porter
  0 siblings, 1 reply; 15+ messages in thread
From: Max Nikulin @ 2022-02-09 16:46 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 3869 bytes --]

On 08/02/2022 17:44, Tianshu Wang wrote:
> Max Nikulin writes:
> 
>> Thank you, such approach, unlike mine example, does not have code 
>> duplication. On the other hand it loads org-protocol on any remote 
>> command, not only for "files" representing org-protocol URIs.
>> Maybe defadvice in org-protocol.el should be changed by newer
>> advice-add with a function containing body of the old advice.
> 
> Yes, I replaced the original code with advice-add (not fully tested).

Sorry, I was not clear enough. I meant a patch similar to the attached 
one. It is not tested for greedy subprotocols. It allows to check 
arguments to decide if it is necessary to load org-protocol:

(defun org-protocol-lazy-load (args)
   (if (or (featurep 'org-protocol)
	  (not (delq nil
		     (mapcar (lambda (loc)
			       ;; loc: (file-name . (line . column))
			       (string-match-p "\\(?:^\\|[/\\\\]\\)org-protocol:" (car loc)))
			     (car args)))))
       args
     (require 'org-protocol)
     (org-protocol-detect-protocol-server args)))

(server-start)

(advice-add 'server-visit-files :filter-args #'org-protocol-lazy-load)


On 08/02/2022 02:06, Jim Porter wrote:
> On 2/7/2022 6:57 AM, Max Nikulin wrote:
>>> Maybe another option would be to add an --apply argument that really 
>>> *does* consume the other command-line args and turns them into a 
>>> properly-quoted function call. Roughly speaking, it would turn this:
>>>
>>>    emacs --apply func foo bar baz
>>>
>>> into this:
>>>
>>>    (apply #'func '(foo bar baz))
>>
>> You have almost managed to sell this idea to me. However even --apply 
>> is just sugar for
>>
>>      emacsclient --eval "(apply #'func command-line-args-left)" --arg 1 2
>>
>> So it is --apply option that may require to write a dedicated function 
>> if --arg is not implemented. Another limitation of --apply is that the 
>> function must accept string arguments only, no lists or even integers 
>> or boolean.
> 
> Yeah, `--arg' does have greater flexibility in that regard, but it comes 
> at the cost of verbosity and some slight behavioral differences with the 
> equivalent in the main emacs executable. For emacs itself, any elements 
> of `command-line-args-left' that aren't consumed by a function will get 
> opened as files, but I don't think that would be the case with 
> emacsclient. Does this difference matter? Probably not. However, 
> avoiding `command-line-args-left' just feels intuitively "cleaner" to 
> me. Still, I think `--arg' should work and be backwards-compatible, so I 
> don't mind it, even if I prefer the simplicity of `--apply'.

It is not a problem to implement --apply in addition to --arg. It may 
just mean
     --eval '(apply-from-command-line-args-left)'

     emacs --batch -l apply-from-command-line-args-left.el \
         --eval '(apply-from-command-line-args-left)' \
         message 'test %S, %S' Hello 'World!'

(defun apply-from-command-line-args-left ()
   (let* ((name (car command-line-args-left))
	 (func (symbol-function (intern name))))
     (if func
       (apply func (cdr command-line-args-left))
      (error (format "Symbol's function definition is void: %s" name)))))

OK, I forgot to set the variable to nil.

> On a related note, there is still an issue with `--eval' in some cases. 
> It fails to work with emacsclient when invoking an alternate editor:
> 
>    emacsclient --alternate-editor emacs --eval '(message "hi")'
> 
> If Emacs isn't already running, that will open a new buffer named 
> '(message "hi")'. I think that's just a bug in how emacsclient handles 
> `--eval', though fixing it would make org-protocol links work more 
> reliably (if they used `--eval' as proposed, that is).

     emacsclient --alternate-editor '' --eval '(message "hi")'

But it makes it harder to use it during debugging when -Q -L 
~/src/org-mode/lisp options are required.

[-- Attachment #2: 0001-org-protocol.el-New-style-advice.patch --]
[-- Type: text/x-patch, Size: 2217 bytes --]

From 8a4f8db239d004513be35249ed71e222381b0162 Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Wed, 9 Feb 2022 23:25:28 +0700
Subject: [PATCH] org-protocol.el: New-style advice

lisp/org-protocol.el (org-protocol-detect-protocol-server): Use
`advice-add' instead of deprecated `defadvice'.

Advice as a function allows to call it from another similar advice
loading `org-protocol' on demand.
---
 lisp/org-protocol.el | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index 7c4de03bc..fb41e3c0f 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -687,25 +687,29 @@ to deal with new-style links.")
                     (throw 'fname t))))))))
       fname)))
 
-(defadvice server-visit-files (before org-protocol-detect-protocol-server activate)
+(defun org-protocol-detect-protocol-server (args)
   "Advice server-visit-flist to call `org-protocol-modify-filename-for-protocol'."
-  (let ((flist (if org-protocol-reverse-list-of-files
-                   (reverse  (ad-get-arg 0))
-                 (ad-get-arg 0)))
-        (client (ad-get-arg 1)))
+  (let* ((files (nth 0 args))
+         (client (nth 1 args))
+         (flist (if org-protocol-reverse-list-of-files
+                    (reverse files)
+                  files)))
     (catch 'greedy
       (dolist (var flist)
 	;; `\' to `/' on windows.  FIXME: could this be done any better?
         (let ((fname  (expand-file-name (car var))))
           (setq fname (org-protocol-check-filename-for-protocol
-		       fname (member var flist)  client))
+		       fname (member var flist) client))
           (if (eq fname t) ;; greedy? We need the t return value.
               (progn
-                (ad-set-arg 0 nil)
+                (setcar args nil)
                 (throw 'greedy t))
             (if (stringp fname) ;; probably filename
                 (setcar var fname)
-              (ad-set-arg 0 (delq var (ad-get-arg 0))))))))))
+              (setcar args (delq var files))))))))
+  args)
+
+(advice-add 'server-visit-files :filter-args #'org-protocol-detect-protocol-server)
 
 ;;; Org specific functions:
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: Lazy load of org-protocol
  2022-02-09 16:46           ` Max Nikulin
@ 2022-02-09 19:22             ` Jim Porter
  2022-02-10 14:44               ` Max Nikulin
  0 siblings, 1 reply; 15+ messages in thread
From: Jim Porter @ 2022-02-09 19:22 UTC (permalink / raw)
  To: emacs-orgmode

On 2/9/2022 8:46 AM, Max Nikulin wrote:
> It is not a problem to implement --apply in addition to --arg.
[snip]

For the purposes of this issue, I think either solution would probably 
work. It probably depends on what people think on emacs-devel.

>> On a related note, there is still an issue with `--eval' in some 
>> cases. It fails to work with emacsclient when invoking an alternate 
>> editor:
>>
>>    emacsclient --alternate-editor emacs --eval '(message "hi")'
>>
>> If Emacs isn't already running, that will open a new buffer named 
>> '(message "hi")'. I think that's just a bug in how emacsclient handles 
>> `--eval', though fixing it would make org-protocol links work more 
>> reliably (if they used `--eval' as proposed, that is).
> 
>      emacsclient --alternate-editor '' --eval '(message "hi")'
> 
> But it makes it harder to use it during debugging when -Q -L 
> ~/src/org-mode/lisp options are required.

Right, invoking the main Emacs instance as a daemon (i.e. when 
--alternate-editor is the empty string) works pretty differently from 
invoking the main Emacs instance directly (i.e. when --alternate-editor 
is "emacs" or somesuch). In the former case, emacsclient starts "emacs 
--daemon" and then tries to reconnect to the daemon; in the latter, it 
just takes the arguments passed to emacsclient and forwards them (well, 
some of them) to emacs. The end result is that it runs "emacs '(message 
"hi")'", which isn't correct.

It'd be nice to fix the behavior of "--alternate-editor emacs" so it 
passes the --eval flag along too. This might be tricky though, since the 
semantics of --eval aren't quite the same for emacs and emacsclient. For 
emacs, --eval means that just the next argument is Lisp code; for 
emacsclient, --eval means that *all* subsequent (positional) arguments 
are Lisp code.

In practice though, I don't think fixing this is actually *required* to 
fix the issue of how org-protocol is handled. It only causes issues for 
the subset of people who use "--alternate-editor emacs" or something 
similar.

- Jim


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Lazy load of org-protocol
  2022-02-09 19:22             ` Jim Porter
@ 2022-02-10 14:44               ` Max Nikulin
  0 siblings, 0 replies; 15+ messages in thread
From: Max Nikulin @ 2022-02-10 14:44 UTC (permalink / raw)
  To: emacs-orgmode

On 10/02/2022 02:22, Jim Porter wrote:
> On 2/9/2022 8:46 AM, Max Nikulin wrote:
> 
>>> On a related note, there is still an issue with `--eval' in some 
>>> cases. It fails to work with emacsclient when invoking an alternate 
>>> editor:
>>>
>>>    emacsclient --alternate-editor emacs --eval '(message "hi")'
>>>
>>> If Emacs isn't already running, that will open a new buffer named 
>>> '(message "hi")'. I think that's just a bug in how emacsclient 
>>> handles `--eval', though fixing it would make org-protocol links work 
>>> more reliably (if they used `--eval' as proposed, that is).
>>
>>      emacsclient --alternate-editor '' --eval '(message "hi")'
>>
>> But it makes it harder to use it during debugging when -Q -L 
>> ~/src/org-mode/lisp options are required.
> 
> Right, invoking the main Emacs instance as a daemon (i.e. when 
> --alternate-editor is the empty string) works pretty differently from 
> invoking the main Emacs instance directly (i.e. when --alternate-editor 
> is "emacs" or somesuch). In the former case, emacsclient starts "emacs 
> --daemon" and then tries to reconnect to the daemon; in the latter, it 
> just takes the arguments passed to emacsclient and forwards them (well, 
> some of them) to emacs. The end result is that it runs "emacs '(message 
> "hi")'", which isn't correct.

It looks like a bug. I believed that other application may be specified 
as --alternate-editor, so --eval arguments should not be passed to it.

Actually I do not like --alternate-editor option at all. It is a single 
string, not an array of executable and options. It allows quotes but 
does not support escaping.

On linux a solution is to start emacs daemon from systemd session as a 
socket-activated application.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] lisp/org-capture.el: Add hook & hook options to org-capture (Valentin Herrmann)
  2022-02-08 19:02 ` [PATCH] lisp/org-capture.el: Add hook & hook options to org-capture (Valentin Herrmann) No Wayman
  2022-02-09  4:10   ` Ihor Radchenko
@ 2022-02-10 19:32   ` Greg Minshall
  1 sibling, 0 replies; 15+ messages in thread
From: Greg Minshall @ 2022-02-10 19:32 UTC (permalink / raw)
  To: No Wayman; +Cc: emacs-orgmode-request, emacs-orgmode

fwiw...

> I've implemented what you're proposing here (and much more) in a 
> package you may find useful a couple years ago. I pitched adopting 
> some of the ideas into org-mode proper and was willing to do the 
> work. My proposal was met with enthusiastic silence:
> 
> https://www.github.com/progfolio/doct

i've been using doct for a while, and have been very happy with it.

(thanks for implementing it.)

cheers, Greg


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] lisp/org-capture.el: Add hook & hook options to org-capture (Valentin Herrmann)
  2022-02-09  7:11     ` No Wayman
@ 2022-03-20 10:43       ` Ihor Radchenko
  0 siblings, 0 replies; 15+ messages in thread
From: Ihor Radchenko @ 2022-03-20 10:43 UTC (permalink / raw)
  To: No Wayman; +Cc: emacs-orgmode-request, emacs-orgmode

No Wayman <iarchivedmywholelife@gmail.com> writes:

>> I think Nicolas gave some reasonable comments, didn't he? He 
>> suggested
>> to incorporate some of the ideas into the existing Org mode 
>> code.
>>
>> https://orgmode.org/list/87wo66t8i7.fsf@gmail.com
>
> I believe you're referring to:
>
> https://list.orgmode.org/87y2qlgq33.fsf@nicolasgoaziou.fr/
>
> He had some reasonable questions about the design, but said he 
> only uses very basic capture templates, and could not comment 
> beyond that on the design. The hope was that people who use more 
> of org-capture's features would chime in. That didn't happen.

I just pinged that thread again. Note that I strongly support adding
doct to Org.

> Of course doct's features could be added to org-capture in a 
> piecemeal fashion, but I'm not too keen on contributing anything 
> non-trivial to Org these days. Too much back and forth coupled 
> with discussion that can stall due to lack of interest. 
> e.g.

> https://list.orgmode.org/87h7e6dluc.fsf@gmail.com/
> https://list.orgmode.org/87tuit73ij.fsf@gmail.com/

I cannot comment on the lexical binding thing. I do not really know
lexical binding internals enough to judge what kind of problem that
patch is trying to solve. However, Bastien commented on the second
patch. I do find it useful as well. I think the only problem with that
patch is that it is not listed in updates.orgmode.org and Bastien missed
your last reply.

Best,
Ihor



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-03-20 10:43 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <mailman.61.1644253327.32758.emacs-orgmode@gnu.org>
2022-02-08 19:02 ` [PATCH] lisp/org-capture.el: Add hook & hook options to org-capture (Valentin Herrmann) No Wayman
2022-02-09  4:10   ` Ihor Radchenko
2022-02-09  7:11     ` No Wayman
2022-03-20 10:43       ` Ihor Radchenko
2022-02-10 19:32   ` Greg Minshall
2022-02-05 11:54 Lazy load of org-protocol Max Nikulin
2022-02-05 18:27 ` Jim Porter
2022-02-06 16:42   ` Max Nikulin
2022-02-06 19:40     ` Jim Porter
2022-02-07 14:57       ` Max Nikulin
2022-02-07 19:06         ` Jim Porter
2022-02-09 16:46           ` Max Nikulin
2022-02-09 19:22             ` Jim Porter
2022-02-10 14:44               ` Max Nikulin
2022-02-08 10:44         ` Emacs-orgmode Digest, Vol 192, Issue 8 Tianshu Wang

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).