* Re: [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential'. [not found] <E1VBinS-0002iC-21@vcs.savannah.gnu.org> @ 2013-08-20 14:35 ` Stefan Monnier 2013-08-20 14:59 ` Michael Albinus 0 siblings, 1 reply; 10+ messages in thread From: Stefan Monnier @ 2013-08-20 14:35 UTC (permalink / raw) To: Michael Albinus; +Cc: emacs-devel > * minibuffer.el (completion--sifn-requote): Bind `non-essential'. This is wrong. completion--sifn-requote is used in many contexts, some of which are "essential" and some not. IOW if you need to bind non-essential, it shouldn't be here, but somewhere higher up the call-chain. Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential'. 2013-08-20 14:35 ` [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential' Stefan Monnier @ 2013-08-20 14:59 ` Michael Albinus 2013-08-20 20:35 ` Stefan Monnier 0 siblings, 1 reply; 10+ messages in thread From: Michael Albinus @ 2013-08-20 14:59 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> * minibuffer.el (completion--sifn-requote): Bind `non-essential'. > > This is wrong. completion--sifn-requote is used in many contexts, some > of which are "essential" and some not. IOW if you need to bind > non-essential, it shouldn't be here, but somewhere higher up the > call-chain. Honestly, I don't know too much about the completion code in minibuffer.el. The OP has reported the following backtrace (shortened by me): substitute-in-file-name("/ssh:vagrant@192.168.10.") completion--sifn-requote(27 "/ssh:vagrant@192.168.10.2:/") completion--twq-try("/ssh:vagrant@192.168.10.2:/" "/ssh:vagrant@192.168.10.2:/" "/ssh:vagrant@192.168.10.2:/" 0 substitute-in-file-name completion--sifn-requote) completion--file-name-table("/ssh:vagrant@192.168.10.2:/" file-exists-p nil) complete-with-action(nil completion--file-name-table "/ssh:vagrant@192.168.10.2:/" file-exists-p) #[257 "\303\302\300\301$\207" ["/ssh:vagrant@192.168.10.2:/" file-exists-p nil complete-with-action] 6 "\n\n(fn TABLE)"](completion--file-name-table) funcall(#[257 "\303\302\300\301$\207" ["/ssh:vagrant@192.168.10.2:/" file-exists-p nil complete-with-action] 6 "\n\n(fn TABLE)"] completion--file-name-table) (setq res (funcall fun (car (prog1 xs (setq xs (cdr xs)))))) (condition-case err (setq res (funcall fun (car (prog1 xs (setq xs (cdr xs)))))) (error (if firsterror nil (setq firsterror err)) nil)) (while (and (not res) xs) (condition-case err (setq res (funcall fun (car (prog1 xs (setq xs (cdr xs)))))) (error (if firsterror nil (setq firsterror err)) nil))) (let ((firsterror nil) res) (while (and (not res) xs) (condition-case err (setq res (funcall fun (car (prog1 xs (setq xs ...))))) (error (if firsterror nil (setq firsterror err)) nil))) (or res (if firsterror (signal (car firsterror) (cdr firsterror))))) completion--some(#[257 "\303\302\300\301$\207" ["/ssh:vagrant@192.168.10.2:/" file-exists-p nil complete-with-action] 6 "\n\n(fn TABLE)"] (completion--embedded-envvar-table completion--file-name-table)) read-file-name-internal("/ssh:vagrant@192.168.10.2:/" file-exists-p nil) try-completion("/ssh:vagrant@192.168.10.2:/" read-file-name-internal file-exists-p) Sadly, I cannot reproduce this scenario locally. And I have no idea, why completion--sifn-requote(27 "/ssh:vagrant@192.168.10.2:/") calls substitute-in-file-name("/ssh:vagrant@192.168.10.") > Stefan Best regards, Michael. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential'. 2013-08-20 14:59 ` Michael Albinus @ 2013-08-20 20:35 ` Stefan Monnier 2013-08-21 9:46 ` Michael Albinus 0 siblings, 1 reply; 10+ messages in thread From: Stefan Monnier @ 2013-08-20 20:35 UTC (permalink / raw) To: Michael Albinus; +Cc: emacs-devel >>> * minibuffer.el (completion--sifn-requote): Bind `non-essential'. >> This is wrong. completion--sifn-requote is used in many contexts, some >> of which are "essential" and some not. IOW if you need to bind >> non-essential, it shouldn't be here, but somewhere higher up the >> call-chain. > Honestly, I don't know too much about the completion code in > minibuffer.el. The OP has reported the following backtrace (shortened by > me): My point is simply that the backtrace you show does not seem to perform non-essential work, so binding non-essential is not right. It is computing the completion of /ssh:vagrant@192.168.10.2:/ (not quite sure why, since the backtrace doesn't go further up). > Sadly, I cannot reproduce this scenario locally. And I have no idea, why > completion--sifn-requote(27 "/ssh:vagrant@192.168.10.2:/") > calls > substitute-in-file-name("/ssh:vagrant@192.168.10.") Can't say offhand either why not. Maybe we should simply demote errors from substitute-in-file-name inside completion--sifn-requote. At the same time I wonder why (substitute-in-file-name "/ssh:vagrant@192.168.10.") should signal an error: it looks like an incomplete filename, so we don't know yet whether it'll really be an error or not. Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential'. 2013-08-20 20:35 ` Stefan Monnier @ 2013-08-21 9:46 ` Michael Albinus 2013-08-21 20:08 ` Stefan Monnier 0 siblings, 1 reply; 10+ messages in thread From: Michael Albinus @ 2013-08-21 9:46 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: Hi Stefan, >> Honestly, I don't know too much about the completion code in >> minibuffer.el. The OP has reported the following backtrace (shortened by >> me): > > My point is simply that the backtrace you show does not seem to perform > non-essential work, so binding non-essential is not right. It is > computing the completion of /ssh:vagrant@192.168.10.2:/ (not quite sure > why, since the backtrace doesn't go further up). The whole backtrace is in <http://lists.gnu.org/archive/html/emacs-devel/2013-08/msg00533.html> (the second backtrace in this message). >> Sadly, I cannot reproduce this scenario locally. And I have no idea, why >> completion--sifn-requote(27 "/ssh:vagrant@192.168.10.2:/") >> calls >> substitute-in-file-name("/ssh:vagrant@192.168.10.") > > Can't say offhand either why not. Maybe we should simply demote errors > from substitute-in-file-name inside completion--sifn-requote. > > At the same time I wonder why (substitute-in-file-name > "/ssh:vagrant@192.168.10.") should signal an error: it looks like an > incomplete filename, so we don't know yet whether it'll really be an > error or not. For Tramp, it is a complete filename wrt syntax. "ssh" is recognised as host name, which is regarded as an error since we had decided this some weeks ago. See <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13900#24>. Setting `non-essential' to t is a mean to tell Tramp not to check this. > Stefan Best regards, Michael. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential'. 2013-08-21 9:46 ` Michael Albinus @ 2013-08-21 20:08 ` Stefan Monnier 2013-08-22 9:50 ` Michael Albinus 0 siblings, 1 reply; 10+ messages in thread From: Stefan Monnier @ 2013-08-21 20:08 UTC (permalink / raw) To: Michael Albinus; +Cc: emacs-devel >> My point is simply that the backtrace you show does not seem to perform >> non-essential work, so binding non-essential is not right. It is >> computing the completion of /ssh:vagrant@192.168.10.2:/ (not quite sure >> why, since the backtrace doesn't go further up). > The whole backtrace is in > <http://lists.gnu.org/archive/html/emacs-devel/2013-08/msg00533.html> > (the second backtrace in this message). So we're in the middle of minibuffer--complete-and-exit which calls try-completion. That's very definitely not "non-essential". >> At the same time I wonder why (substitute-in-file-name >> "/ssh:vagrant@192.168.10.") should signal an error: it looks like an >> incomplete filename, so we don't know yet whether it'll really be an >> error or not. > For Tramp, it is a complete filename wrt syntax. "ssh" is recognised as > host name, which is regarded as an error since we had decided this some > weeks ago. See <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13900#24>. Yes, it could be a "complete but ambiguous/erroneous" file name, but it could also be a "not yet complete file name". So maybe, if there's no ":" nor "/" after "/ssh:", we should refrain from signaling an error. > Setting `non-essential' to t is a mean to tell Tramp not to check this. I do know that, but that's just an ugly hack here, since the code is not non-essential. Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential'. 2013-08-21 20:08 ` Stefan Monnier @ 2013-08-22 9:50 ` Michael Albinus 2013-08-22 15:39 ` Stefan Monnier 0 siblings, 1 reply; 10+ messages in thread From: Michael Albinus @ 2013-08-22 9:50 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >>> My point is simply that the backtrace you show does not seem to perform >>> non-essential work, so binding non-essential is not right. It is >>> computing the completion of /ssh:vagrant@192.168.10.2:/ (not quite sure >>> why, since the backtrace doesn't go further up). >> The whole backtrace is in >> <http://lists.gnu.org/archive/html/emacs-devel/2013-08/msg00533.html> >> (the second backtrace in this message). > > So we're in the middle of minibuffer--complete-and-exit which calls > try-completion. That's very definitely not "non-essential". Not `minibuffer--complete-and-exit'. But the completion functions I would call non-essential. As said, I don't know too much about completion code in minibuffer.el (and the functions' docstrings don't help much). Maybe `completion--sifn-requote' is not suited to bind `non-essential'. But there are other `completion--*' functions in the backtrace, up to `minibuffer--complete-and-exit'. >>> At the same time I wonder why (substitute-in-file-name >>> "/ssh:vagrant@192.168.10.") should signal an error: it looks like an >>> incomplete filename, so we don't know yet whether it'll really be an >>> error or not. >> For Tramp, it is a complete filename wrt syntax. "ssh" is recognised as >> host name, which is regarded as an error since we had decided this some >> weeks ago. See <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13900#24>. > > Yes, it could be a "complete but ambiguous/erroneous" file name, but it > could also be a "not yet complete file name". So maybe, if there's no > ":" nor "/" after "/ssh:", we should refrain from signaling an error. Hmm. Why not signalling a pilot error, when a user applies "C-x f /sudo: RET"? It's not so different from the case the user has described in Bug#13900, which was the trigger to introduce the rule "host name != any method name". >> Setting `non-essential' to t is a mean to tell Tramp not to check this. > > I do know that, but that's just an ugly hack here, since the code is not > non-essential. I don't know which further plans you have with `non-essential'. But its setting to t is acknowledged so far only in ange-ftp and Tramp. And it is bound to t only in icomplete.el, ido.el and rfn-eshadow.el, as yet. For exactly that reason: "please relax your tests and connection actions when performing file name completion or minibuffer decoration". That doesn't sound ugly to me. > Stefan Best regards, Michael. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential'. 2013-08-22 9:50 ` Michael Albinus @ 2013-08-22 15:39 ` Stefan Monnier 2013-08-22 18:40 ` Michael Albinus 0 siblings, 1 reply; 10+ messages in thread From: Stefan Monnier @ 2013-08-22 15:39 UTC (permalink / raw) To: Michael Albinus; +Cc: emacs-devel >> So we're in the middle of minibuffer--complete-and-exit which calls >> try-completion. That's very definitely not "non-essential". > Not `minibuffer--complete-and-exit'. But the completion functions I > would call non-essential. Huh? You did notice that the command's name has "-complete-" in it, right? > As said, I don't know too much about completion code in minibuffer.el > (and the functions' docstrings don't help much). Maybe > `completion--sifn-requote' is not suited to bind `non-essential'. But > there are other `completion--*' functions in the backtrace, up to > `minibuffer--complete-and-exit'. They're all just computing the completion. >> Yes, it could be a "complete but ambiguous/erroneous" file name, but it >> could also be a "not yet complete file name". So maybe, if there's no >> ":" nor "/" after "/ssh:", we should refrain from signaling an error. > Hmm. Why not signalling a pilot error, when a user applies "C-x > f /sudo: RET"? It's not so different from the case the user has > described in Bug#13900, which was the trigger to introduce the rule > "host name != any method name". Then how about a different take on the problem: Tramp shouldn't signal an "ambiguous file name" error when its behavior would be the same either way. AFAIK substitute-in-file-name would return the same result regardless of whether "/ssh:vagrant@192.168.10." is taken to be the file "vagrant@192.168.10." in host "ssh" or access via ssh to host "192.168.10.". > I don't know which further plans you have with `non-essential'. But its > setting to t is acknowledged so far only in ange-ftp and Tramp. So far. > And it is bound to t only in icomplete.el, ido.el and rfn-eshadow.el, as > yet. For exactly that reason: "please relax your tests and connection > actions when performing file name completion or minibuffer decoration". The key word is "decoration" and not "completion". The present case is about completion and not about decoration. > That doesn't sound ugly to me. It's called `non-essential' and not `tramp-inhibit' specifically because I don't want icomplete.el and friends to have Tramp-specific hacks in them. If you start abusing it on the premise that "that's how I use it in Tramp", then it defeats the whole purpose. Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential'. 2013-08-22 15:39 ` Stefan Monnier @ 2013-08-22 18:40 ` Michael Albinus 2013-08-23 3:21 ` Stefan Monnier 0 siblings, 1 reply; 10+ messages in thread From: Michael Albinus @ 2013-08-22 18:40 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> Hmm. Why not signalling a pilot error, when a user applies "C-x >> f /sudo: RET"? It's not so different from the case the user has >> described in Bug#13900, which was the trigger to introduce the rule >> "host name != any method name". > > Then how about a different take on the problem: Tramp shouldn't signal > an "ambiguous file name" error when its behavior would be the same > either way. AFAIK substitute-in-file-name would return the same result > regardless of whether "/ssh:vagrant@192.168.10." is taken to be the file > "vagrant@192.168.10." in host "ssh" or access via ssh to host > "192.168.10.". Well, I could try to implement it in Tramp's `substitute-in-file-name' handler. However, I don't know which other functions with file name handlers are called when applying `minibuffer--complete-and-exit'. Let's see. (But it would be a just a hack) >> And it is bound to t only in icomplete.el, ido.el and rfn-eshadow.el, as >> yet. For exactly that reason: "please relax your tests and connection >> actions when performing file name completion or minibuffer decoration". > > The key word is "decoration" and not "completion". The present case is > about completion and not about decoration. > >> That doesn't sound ugly to me. > > It's called `non-essential' and not `tramp-inhibit' specifically because > I don't want icomplete.el and friends to have Tramp-specific hacks > in them. If you start abusing it on the premise that "that's how I use > it in Tramp", then it defeats the whole purpose. I don't want to spread trampish functionality somewhere else. But Tramp is broken by design: its syntax is ambigous, and sometimes Tramp cannot decide merely on the file name syntax whether this is the intended file name to be used on the wire. That's why it needs advice from the caller: "don't take this file name serious, I'm not interested in a working connection yet". That does not mean that Tramp wouldn't use an established connection if it exists already, it simply means that Tramp doesn't open a new connection if there isn't one. Minibuffer decoration is such a case where Tramp needs such an advice, and file name completion is another case. An incomplete file name does not need a working connection. Consequently, some predicates used in file name completion (like `file-exists-p') will give proper results only when the connection is established finally. But that might be acceptable. `non-essential' bound to t looks to me perfect for such an advice. If you believe it is an abuse of that variable, please give me a hint how Tramp could get such an advice otherwise. It is not "that's how I use it in Tramp". Since I hack for Tramp (11 years these days) this problem eats a serious amount of my maintenance time. I would be more than happy if it could be solved finally, somehow. Due to the misdesigned Tramp file name syntax, I don't see how to solve it inside Tramp. > Stefan Best regards, Michael. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential'. 2013-08-22 18:40 ` Michael Albinus @ 2013-08-23 3:21 ` Stefan Monnier 2013-08-26 13:19 ` Michael Albinus 0 siblings, 1 reply; 10+ messages in thread From: Stefan Monnier @ 2013-08-23 3:21 UTC (permalink / raw) To: Michael Albinus; +Cc: emacs-devel > Well, I could try to implement it in Tramp's `substitute-in-file-name' > handler. However, I don't know which other functions with file name > handlers are called when applying `minibuffer--complete-and-exit'. > Let's see. (But it would be a just a hack) If you only do it in substitute-in-file-name it will be a hack, indeed. But if you do it by moving the check to a place where the ambiguity has consequences, I think it would not be a hack. > Minibuffer decoration is such a case where Tramp needs such an advice, > and file name completion is another case. I don't see why you put completion in the same camp as decorations. Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential'. 2013-08-23 3:21 ` Stefan Monnier @ 2013-08-26 13:19 ` Michael Albinus 0 siblings, 0 replies; 10+ messages in thread From: Michael Albinus @ 2013-08-26 13:19 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > If you only do it in substitute-in-file-name it will be a hack, indeed. > But if you do it by moving the check to a place where the ambiguity has > consequences, I think it would not be a hack. Finally, I have reverted the fix in minibuffer.el. In Tramp, the check for a proper host name is delayed until the connection is opened. I'm still not convinced, that Tramp doesn't need more indications for `non-essential'. But let's wait until it is reported by users. > Stefan Best regards, Michael. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-08-26 13:19 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <E1VBinS-0002iC-21@vcs.savannah.gnu.org> 2013-08-20 14:35 ` [Emacs-diffs] trunk r113958: * minibuffer.el (completion--sifn-requote): Bind `non-essential' Stefan Monnier 2013-08-20 14:59 ` Michael Albinus 2013-08-20 20:35 ` Stefan Monnier 2013-08-21 9:46 ` Michael Albinus 2013-08-21 20:08 ` Stefan Monnier 2013-08-22 9:50 ` Michael Albinus 2013-08-22 15:39 ` Stefan Monnier 2013-08-22 18:40 ` Michael Albinus 2013-08-23 3:21 ` Stefan Monnier 2013-08-26 13:19 ` Michael Albinus
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.