From: Gustavo Barros <firstname.lastname@example.org> To: email@example.com Subject: Re: Bug: Double trailing slash for default candidate in org-refile-get-target [9.4 (9.4-7-g3eccc5-elpaplus @ /home/gustavo/.emacs.d/elpa/org-plus-contrib-20200921/)] Date: Sun, 14 Feb 2021 09:55:51 -0300 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> Hi All, I'd like to kindly bump this report. It is an issue which has been around for some time. The report provides a clear reproduction recipe, which stardiviner was able to reproduce, and still affects current "org-plus-contrib-20210208". The report also provides a hopefully convincing code analysis of the affected function `org-refile-get-location' and a suggested fix (I just don't send a patch, because I can't provide CA). Best regards, Gustavo. On Mon, 21 Sep 2020 at 15:34, Gustavo Barros <firstname.lastname@example.org> wrote: > Hi All, > > some time ago, I've reported an issue regarding duplicity of the > default > candidate in `org-refile' > (https://email@example.com/). The problem was > that, > when using `org-refile-use-outline-path' an "extra" slash was appended > at the > end of every path, but candidates were stored in `org-refile-history' > without > that extra slash. Bastien took care of that and indeed changed things > so as > to pass the elements to `org-refile-history' with the trailing slash > as > appropriate. > > At the time, I reported a difference of behavior between > `completing-read-default' and `ivy-completing-read' after the > mentioned > commit by Bastien. But the issue did not appear for Bastien, which > does not > use Ivy, and I also was not able to diagnose the problem properly. I > felt I > didn't have enough to offer as to insist, so I resorted to an old hack > of > mine. But the new release this week (thank you very much!, btw) has > bitten me > again on this, so I went back to some digging, and hopefully I can do > a better > job this time in diagnosing the problem and suggesting a fix. > > > An ECM to reproduce the issue as it currently stands is: > > - Start 'emacs -Q' > > - Do an initial setup: > #+begin_src emacs-lisp > (add-to-list 'load-path "~/.emacs.d/elpa/org-plus-contrib-20200921") > (add-to-list 'load-path "~/.emacs.d/elpa/ivy-20200826.955") > ;; Those are the latest Org weekly build (Org 9.4) and the current up > to > date > ;; Ivy at Melpa > (setq org-refile-targets '(("~/org/test.org" :maxlevel . 2))) > (setq org-refile-use-outline-path 'file) > (setq org-outline-path-complete-in-steps nil) > (require 'ivy) > (ivy-mode) > ;; Bear with me, the problem is not with Ivy, I'll demonstrate that. > #+end_src > > - Open file "~/org/test.org", with contents: > #+begin_src org > ,* Top heading 1 > ,* Top heading 2 > ,** Entry 1 > ,** Entry 2 > #+end_src > > - Go to heading "Entry 1", refile it to "Top heading 1" > > - Go to heading "Entry 2", and call `org-refile' > > - Observe the available candidates, and notice "test.org/Top heading > 1/" is > there twice, once as the default candidate, with a *double* > trailing slash, > and also on the paths list, with a single trailing slash. > > > I've tried to pin down what is going on here and my understanding is > that > Bastien's fix on that previous thread did indeed correct the problem > of the > missing trailing slash in `org-refile-history' and this indeed > corresponds > correctly to the state of the completion "collection" (the let bound > `tbl' > variable in `org-refile-get-location'), as it should. But there > remained a > couple of instances in `org-refile-get-location' which added the > trailing > slash considering `org-refile-history' didn't have them, so that when > this is > done, we get a double trailing slash. > > The two instances are: 1) when the completion function is actually > called: > > #+begin_src emacs-lisp > (setq answ (funcall cfunc prompt tbl nil (not new-nodes) > nil 'org-refile-history > (or cdef (concat (car org-refile-history) > extra)))) > #+end_src > > 2) And three lines bellow, on the let binding: > > #+begin_src emacs-lisp > (let* ((last-refile-loc (car org-refile-history)) > (last-refile-loc-path (concat last-refile-loc extra))) > ...) > #+end_src > > In both instances, we are getting the `car' of `org-refile-history' > which now > already has `extra' (that is, the trailing slash) and adding it again. > > My suggested fix is to remove these `extra's in duplicity, they are > remnants > of when `org-refile-history' didn't have them already. That is: > > #+begin_src emacs-lisp > (setq answ (funcall cfunc prompt tbl nil (not new-nodes) > nil 'org-refile-history > (or cdef (car org-refile-history)))) > #+end_src > > And: > > #+begin_src emacs-lisp > (let* ((last-refile-loc (car org-refile-history)) > (last-refile-loc-path last-refile-loc)) > ...) > #+end_src > > Of course, the second opens some opportunity to simplify the code that > follows > considering `last-refile-loc-path' and `last-refile-loc' are now > identical. > > > Why I think this is the problem and the correct way to fix it: > > 1) If you add inspection points at the appropriate locations for the > sexps > =(concat (car org-refile-history) extra)= and =(concat last-refile-loc > extra)= > you will find the double trailing slash there, and it shouldn't be > there. > > 2) The visual manifestation of this double trailing slash in the > default > candidate with `ivy-mode' is there therefore independently of > `ivy-mode`. Indeed, `ivy-mode' basically sets > `completing-read-function' to > `ivy-completing-read', which in turn calls its main API function > `ivy-read'. > `ivy-completing-read' just passes along the `def' argument without > manipulating it. As far as I can see, `ivy-read' also does not change > it > before calling `read-from-minibuffer'. > > Why `completing-read-default' does not show this second trailing slash > of the > `def' argument it received is another matter. But it's there... > > 3) I've tested this suggested fix in the following scenarios (starting > from > the ECM above): > > #+begin_src emacs-lisp > (setq org-refile-use-outline-path 'file) > (setq org-outline-path-complete-in-steps nil) > ;; (ivy-mode) > #+end_src > > #+begin_src emacs-lisp > (setq org-refile-use-outline-path 'file) > (setq org-outline-path-complete-in-steps nil) > (ivy-mode) > #+end_src > > #+begin_src emacs-lisp > (setq org-refile-use-outline-path 'file) > (setq org-outline-path-complete-in-steps t) > ;; `org-olpath-completing-read' does not play well with `ivy-mode' so > no > need > ;; to use it here > #+end_src > > #+begin_src emacs-lisp > (setq org-refile-use-outline-path nil) > (ivy-mode) > #+end_src > > #+begin_src emacs-lisp > (setq org-refile-use-outline-path nil) > ;; (ivy-mode) > #+end_src > > Other non-nil values of `org-refile-use-outline-path' should be > equivalent to > `file' with respect to the issue at hand, so they are implicitly > covered. > > Having tested these cases, to the best of my knowledge, they all work > as > expected. Though I'm admittedly less acquainted with =(setq > org-refile-use-outline-path nil)= and =(setq > org-outline-path-complete-in-steps t)=. > > All tests and ECM were ran with "Org mode version 9.4 > (9.4-7-g3eccc5-elpaplus > @ /home/gustavo/.emacs.d/elpa/org-plus-contrib-20200921/)" and "GNU > Emacs > 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo > version > 1.16.0) of 2020-08-11". > > I hope this is a more useful report than last time. > > > Best, > Gustavo. > > > > > > Emacs : GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version > 3.24.20, > cairo version 1.16.0) > of 2020-08-11 > Package: Org mode version 9.4 (9.4-7-g3eccc5-elpaplus @ > /home/gustavo/.emacs.d/elpa/org-plus-contrib-20200921/) > > current state: > ============== > (setq > org-src-mode-hook '(org-src-babel-configure-edit-buffer > org-src-mode-configure-edit-buffer) > org-link-shell-confirm-function 'yes-or-no-p > org-metadown-hook '(org-babel-pop-to-session-maybe) > org-clock-out-hook '(org-clock-remove-empty-clock-drawer) > org-refile-targets '(("~/org/test.org" :maxlevel . 2)) > org-mode-hook '(#[0 "\300\301\302\303\304$\207" > [add-hook change-major-mode-hook org-show-all append > local] > 5] > #[0 "\300\301\302\303\304$\207" > [add-hook change-major-mode-hook > org-babel-show-result-all > append local] > 5] > org-babel-result-hide-spec org-babel-hide-all-hashes > org-eldoc-load) > org-outline-path-complete-in-steps nil > org-archive-hook '(org-attach-archive-delete-maybe) > org-confirm-elisp-link-function 'yes-or-no-p > org-agenda-before-write-hook '(org-agenda-add-entry-text) > org-metaup-hook '(org-babel-load-in-session-maybe) > org-bibtex-headline-format-function #[257 "\300\236A\207" [:title] 3 > "\n\n(fn > ENTRY)"] > org-babel-pre-tangle-hook '(save-buffer) > org-tab-first-hook '(org-babel-hide-result-toggle-maybe > org-babel-header-arg-expand) > org-agenda-loop-over-headlines-in-active-region nil > org-src-lang-modes '(("arduino" . arduino) ("redis" . redis) ("php" > . php) > ("C" . c) ("C++" . c++) ("asymptote" . asy) > ("bash" . sh) ("beamer" . latex) ("calc" > . fundamental) > ("cpp" . c++) ("ditaa" . artist) ("dot" > . fundamental) > ("elisp" . emacs-lisp) ("ocaml" . tuareg) > ("screen" . shell-script) ("shell" . sh) > ("sqlite" . sql)) > org-occur-hook '(org-first-headline-recenter) > org-cycle-hook '(org-cycle-hide-archived-subtrees > org-cycle-hide-drawers > org-cycle-show-empty-lines > org-optimize-window-after-visibility-change) > org-speed-command-hook '(org-speed-command-activate > org-babel-speed-command-activate) > org-refile-use-outline-path 'file > org-export-before-parsing-hook '(org-attach-expand-links) > org-confirm-shell-link-function 'yes-or-no-p > org-link-parameters '(("attachment" :follow org-attach-follow > :complete > org-attach-complete-link) > ("id" :follow org-id-open) > ("eww" :follow org-eww-open :store > org-eww-store-link) > ("rmail" :follow org-rmail-open :store > org-rmail-store-link) > ("mhe" :follow org-mhe-open :store > org-mhe-store-link) > ("irc" :follow org-irc-visit :store > org-irc-store-link > :export org-irc-export) > ("info" :follow org-info-open :export > org-info-export > :store org-info-store-link) > ("gnus" :follow org-gnus-open :store > org-gnus-store-link) > ("docview" :follow org-docview-open :export > org-docview-export :store > org-docview-store-link) > ("bibtex" :follow org-bibtex-open :store > org-bibtex-store-link) > ("bbdb" :follow org-bbdb-open :export > org-bbdb-export > :complete org-bbdb-complete-link :store > org-bbdb-store-link) > ("w3m" :store org-w3m-store-link) ("file+sys") > ("file+emacs") ("shell" :follow > org-link--open-shell) > ("news" :follow > #[514 "\301\300\302Q\"\207" > ["news" browse-url ":"] 6 "\n\n(fn URL ARG)"] > ) > ("mailto" :follow > #[514 "\301\300\302Q\"\207" > ["mailto" browse-url ":"] 6 "\n\n(fn URL > ARG)"] > ) > ("https" :follow > #[514 "\301\300\302Q\"\207" > ["https" browse-url ":"] 6 "\n\n(fn URL ARG)"] > ) > ("http" :follow > #[514 "\301\300\302Q\"\207" > ["http" browse-url ":"] 6 "\n\n(fn URL ARG)"] > ) > ("ftp" :follow > #[514 "\301\300\302Q\"\207" ["ftp" browse-url > ":"] > 6 "\n\n(fn URL ARG)"] > ) > ("help" :follow org-link--open-help) > ("file" :complete org-link-complete-file) > ("elisp" :follow org-link--open-elisp) > ("doi" :follow org-link--open-doi)) > org-link-elisp-confirm-function 'yes-or-no-p > )
next prev parent reply other threads:[~2021-02-14 12:57 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-21 18:34 Gustavo Barros 2020-09-23 14:17 ` stardiviner 2020-09-23 14:49 ` Gustavo Barros 2021-02-14 12:55 ` Gustavo Barros [this message] 2021-05-04 6:27 ` Bastien 2021-05-10 18:04 Bhavin Gandhi 2021-05-23 18:05 ` Bhavin Gandhi 2021-05-23 18:23 ` Timothy 2021-05-23 18:58 ` Gustavo Barros 2021-05-24 16:34 ` Bhavin Gandhi 2021-05-24 16:58 ` Gustavo Barros
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: https://www.orgmode.org/ * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: Bug: Double trailing slash for default candidate in org-refile-get-target [9.4 (9.4-7-g3eccc5-elpaplus @ /home/gustavo/.emacs.d/elpa/org-plus-contrib-20200921/)]' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this 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).