From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id j9crAkAeKWDzZgAA0tVLHw (envelope-from ) for ; Sun, 14 Feb 2021 12:57:36 +0000 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id ECTjOD8eKWBHcQAAbx9fmQ (envelope-from ) for ; Sun, 14 Feb 2021 12:57:35 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id B855627276 for ; Sun, 14 Feb 2021 13:57:34 +0100 (CET) Received: from localhost ([::1]:44936 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lBGxx-0000GU-NR for larch@yhetil.org; Sun, 14 Feb 2021 07:57:33 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:42700) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lBGwT-0007ym-0X for emacs-orgmode@gnu.org; Sun, 14 Feb 2021 07:56:01 -0500 Received: from mail-qk1-x732.google.com ([2607:f8b0:4864:20::732]:36773) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lBGwQ-0004QN-E2 for emacs-orgmode@gnu.org; Sun, 14 Feb 2021 07:56:00 -0500 Received: by mail-qk1-x732.google.com with SMTP id v206so4165376qkb.3 for ; Sun, 14 Feb 2021 04:55:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:references:user-agent:from:to:subject:in-reply-to:message-id :date:mime-version; bh=qcmqVmc+XMIf4etS1C9dGloOILK+DjAO10KLWV1aGmQ=; b=WxiinOD1XLjLqN1sqbnARjoiyRsufXMafFOz1x84HS8ZcTknl4YUpDoc2ezolaK+hB N20SOnBPXdwgDhKPffIyUQltM9rUlxNxoY4S4LkrH82hfpcxD96Sqzge5BJausjFg8VC otVBOy8T+1ubjM2/AH2Af4g6H1LoHKZ1EHYA2hmp3R6OJcJfl48Xadp1yILHvWpnKjbc kc7o8iXRjP/EfTFqDkBnQD6lfvIadLMNIhk1o6TULEZjsPmfZxgHz3SbfANGc91jvBR4 YaoRkVrAM/qomUhu/4YCR25WI+8pYV8rrS0EIX18qMfM5/VqgtDKs1nz6VBro/disoQE Y79w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:references:user-agent:from:to:subject :in-reply-to:message-id:date:mime-version; bh=qcmqVmc+XMIf4etS1C9dGloOILK+DjAO10KLWV1aGmQ=; b=LA/iZDqWJkp437kmUnt1r9Er0//z3ch/DMra2o3mi9bbkeN4qDtPV4PlIv7qLlwKDn 1yiLMP5WVh1w+4QihIfBdxZnrjQO/Ew0sVs8fkMxQ4vHp/htiexRTjPQBC3+cy1LDA/G T/AMlLIzAWPPCuuQRreolCDAusU+6+g7raNzmydEr40Tyi+iLSFhkGliTXwk8rO700ns duBmw8DPh/lIv9EvBFwWJ8rFYbCmXOzTW7u90daiaG8hX4rWqEbivv6N7hK5jEf4cXYe S62QlD3kefOK/pIxO8uBsnwo40eWo3gGqCOEyGhk7D0NglP/z2rhmsqaCKQ8YKRzeVCL 82tA== X-Gm-Message-State: AOAM530OmtR/rGnNPKK/4m4o+UgPX1YUoHUxAKR/kmew/q4DjNwKkbpd hmVUEcqrnd0QHioaaYB2fOaC4mj8Fny3oA== X-Google-Smtp-Source: ABdhPJydc2Mbr7HY1vIHfsg288vFkJeWw6204f1Ir6k9jTMAuXnWvZntkamX2ME+mVDoo4xWEXHwKA== X-Received: by 2002:a37:e20b:: with SMTP id g11mr10578111qki.292.1613307355828; Sun, 14 Feb 2021 04:55:55 -0800 (PST) Received: from gusbrs-laptop ([154.21.23.155]) by smtp.gmail.com with ESMTPSA id k187sm10254614qkc.74.2021.02.14.04.55.54 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 14 Feb 2021 04:55:55 -0800 (PST) References: <87tuvrj7ww.fsf@gmail.com> User-agent: mu4e 1.4.15; emacs 27.1.91 From: Gustavo Barros To: emacs-orgmode@gnu.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/)] In-reply-to: <87tuvrj7ww.fsf@gmail.com> Message-ID: <87k0radd0o.fsf@gmail.com> Date: Sun, 14 Feb 2021 09:55:51 -0300 MIME-Version: 1.0 Content-Type: text/plain; format=flowed Received-SPF: pass client-ip=2607:f8b0:4864:20::732; envelope-from=gusbrs.2016@gmail.com; helo=mail-qk1-x732.google.com X-Spam_score_int: -17 X-Spam_score: -1.8 X-Spam_bar: - X-Spam_report: (-1.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_ENVFROM_END_DIGIT=0.25, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Migadu-Flow: FLOW_IN X-Migadu-Spam-Score: -1.26 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20161025 header.b=WxiinOD1; dmarc=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Migadu-Queue-Id: B855627276 X-Spam-Score: -1.26 X-Migadu-Scanner: scn1.migadu.com X-TUID: c7azDWvGMCQu 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 wrote: > Hi All, > > some time ago, I've reported an issue regarding duplicity of the > default > candidate in `org-refile' > (https://orgmode.org/list/87lftw1k2n.fsf@gmail.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 > )