From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kyle Meyer Subject: Re: [PATCH 1/2] org-refile: escape slashes only in headline of refile target Date: Fri, 05 May 2017 19:22:52 -0400 Message-ID: <87vapesxbn.fsf@kyleam.com> References: <20170503175003.22151-2-seb@wirrsal.net> <20170503203046.31610-1-seb@wirrsal.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:39093) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d6mYr-0004hR-MH for emacs-orgmode@gnu.org; Fri, 05 May 2017 19:22:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d6mYo-0005Wr-JO for emacs-orgmode@gnu.org; Fri, 05 May 2017 19:22:57 -0400 Received: from pb-smtp2.pobox.com ([64.147.108.71]:58925 helo=sasl.smtp.pobox.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d6mYo-0005WB-DY for emacs-orgmode@gnu.org; Fri, 05 May 2017 19:22:54 -0400 In-Reply-To: <20170503203046.31610-1-seb@wirrsal.net> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: Sebastian =?utf-8?Q?Reu=C3=9Fe?= Cc: emacs-orgmode@gnu.org Sebastian Reu=C3=9Fe 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=E2=80=99t result in any confusion and serve= s 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 defaul= t-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 >=3D 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))) > "/")))) --=20 Kyle