Re: [O] [PATCH 1/2] org-refile: escape slashes only in headline of refile target

[Kyle Meyer]

Sebastian Reuße <address@hidden> writes:

> * org.el (org-refile-get-targets): only escape slashes in headline

For style consistency, the first word following the colon should be
capitalized.  The same applies to the commit subject.

> part of refile target; leave any file-system path component (when
> enabled) unescaped.
>
> The reason to escape slashes in refile targets is to make it clear
> when a slash was part of a headline vs. part of the outline path.  It
> makes sense to treat slashes in the file system part the same way as
> outline paths, since this won’t result in any confusion and serves to
> make target selection less noisy.

That sounds reasonable to me.

> ---
>  lisp/org.el | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)

Based on the Org contributor page [*], it doesn't look like you've
signed papers with the FSF, so the commit message should label this as a
"TINYCHANGE".  (I think this and the next patch would be small enough to
mark as tiny changes.)

[*] http://orgmode.org/worg/org-contribute.html

> diff --git a/lisp/org.el b/lisp/org.el
> index 0d83b4cbd..ad92f3b2e 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -11575,18 +11575,17 @@ (defun org-refile-get-targets (&optional 
> default-buffer)
>                                       (regexp-quote heading)))
>                           (target
>                            (if (not org-refile-use-outline-path) heading
> -                            (mapconcat
> -                             #'org-protect-slash
> -                             (append
> +                            (string-join

Org's master branch currently supports Emacs >= 24.3, but string-join
was added in the 24.5 release.

> +                             (cons
>                                (pcase org-refile-use-outline-path
> -                                (`file (list (file-name-nondirectory
> -                                              (buffer-file-name
> -                                               (buffer-base-buffer)))))
> -                                (`full-file-path
> -                                 (list (buffer-file-name
> -                                        (buffer-base-buffer))))
> -                                (_ nil))
> -                              (org-get-outline-path t t))
> +                                (`file (file-name-nondirectory
> +                                        (buffer-file-name
> +                                         (buffer-base-buffer))))
> +                                (`full-file-path (buffer-file-name
> +                                                  (buffer-base-buffer)))
> +                                (_ ""))

I'm fine with the append -> cons conversion, but avoiding it would make
for a cleaner diff that's more focused on the main purpose of this
patch.

Also, it does slightly change the result because the fall-through case
would now include "/" at the start of the path, but I don't think that's
a big deal given that this case shouldn't be reached if the user sets
org-refile-use-outline-path to a valid value.  Which makes me wonder
whether we should just raise a user-error for this fall-through case.

> +                              (mapcar #'org-protect-slash
> +                                      (org-get-outline-path t t)))
>                               "/"))))

-- 
Kyle