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

[Kyle Meyer <kyle@kyleam.com>]

Sebastian Reuße <seb@wirrsal.net> 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