* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el [not found] ` <20180922154640.9D58220310@vcs0.savannah.gnu.org> @ 2018-12-26 3:34 ` Dmitry Gutov 2018-12-26 20:13 ` Stefan Monnier 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2018-12-26 3:34 UTC (permalink / raw) To: emacs-devel, Stefan Monnier Hi Stefan, I've only noticed this change recently, hence this late email. On 22.09.2018 18:46, Stefan Monnier wrote: > +(cl-defgeneric project-files (project &optional dirs) > + "Return a list of files in directories DIRS in PROJECT. > +DIRS is a list of absolute directories; it should be some > +subset of the project roots and external roots." > + ;; This default implementation only works if project-file-completion-table > + ;; returns a "flat" completion table. That sounds like a fair concern, never thought about it. Do we want to have both as generic functions, though? People will have to take care to keep them in sync. > + ;; FIXME: Maybe we should do the reverse: implement the default > + ;; `project-file-completion-table' on top of `project-files'. Originally I figured that having the completion table be a basic part of the propocol gives some benefits: * If there's a background process that filters files faster than Emacs, then it could actually provide faster file completion. * Completion table is a "lazy" value, which can be handy. Not sure if item 1 is ever going to materialize, and it would only help in certain cases. But since file listing can be slow sometimes, should we consider having some other kind of return value, for performance? Say, a stream of some kind. It could be handy for multifile-based commands, since they only show one file at a time. Ideally, though, the stream should be easily composable with external tools like Grep. Ideas? > + (all-completions > + "" (project-file-completion-table > + project (or dirs (project-roots project))))) > + > (defgroup project-vc nil > "Project implementation using the VC package." > :version "25.1" > @@ -389,12 +401,17 @@ recognized." > ;; removing it when it has no matches. Neither seems natural > ;; enough. Removal is confusing; early expansion makes the prompt > ;; too long. > - (let* ((new-prompt (if default > + (let* (;; (initial-input > + ;; (let ((common-prefix (try-completion "" collection))) > + ;; (if (> (length common-prefix) 0) > + ;; (file-name-directory common-prefix)))) Interesting suggestion if we only want to use this function in project-find-file. I see the same effect, though, if I just press TAB. Or I can right away type a unique file name, press TAB and see it completed to the full file name. I wonder what other people think; I still haven't managed to get off Ido, personally. > +;;;###autoload > +(defun project-search (regexp) > +;;;###autoload > +(defun project-query-replace (from to) I'm not a fan of these names. I know they kind of mirror what we already have in the dired- namespace, but can't we do better? Maybe rename dired-do-search and dired-do-query-replace-regexp later as well. I think it's going to be hard for a user to differentiate between project-find-regexp and project-search. And that they might also have a guess that the latter "searches" for a project among the opened ones. Should we move both commands to the multifile package and name them, for instance, multifile-project-find-regexp (or multifile-project-search) and multifile-project-query-replace-regexp? Originally we thought that this kind of UI preference would be enacted using xref-show-xrefs-function, but apparently that's not so easy to do. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-26 3:34 ` [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el Dmitry Gutov @ 2018-12-26 20:13 ` Stefan Monnier 2018-12-27 1:49 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2018-12-26 20:13 UTC (permalink / raw) To: emacs-devel >> +(cl-defgeneric project-files (project &optional dirs) >> + "Return a list of files in directories DIRS in PROJECT. >> +DIRS is a list of absolute directories; it should be some >> +subset of the project roots and external roots." >> + ;; This default implementation only works if project-file-completion-table >> + ;; returns a "flat" completion table. > > That sounds like a fair concern, never thought about it. Do we want to have > both as generic functions, though? People will have to take care to keep > them in sync. Not sure what you mean by "keep them in sync". > Not sure if item 1 is ever going to materialize, and it would only help in > certain cases. But since file listing can be slow sometimes, should we > consider having some other kind of return value, for performance? > Say, a stream of some kind. It could be handy for multifile-based commands, > since they only show one file at a time. Ideally, though, the stream should > be easily composable with external tools like Grep. > > Ideas? We could define it to return a *sequence*, so it can either return a list, or a stream, or an array, .. >> - (let* ((new-prompt (if default >> + (let* (;; (initial-input >> + ;; (let ((common-prefix (try-completion "" collection))) >> + ;; (if (> (length common-prefix) 0) >> + ;; (file-name-directory common-prefix)))) > > Interesting suggestion if we only want to use this function in > project-find-file. I see the same effect, though, if I just press TAB. Having to type TAB makes the interface quite different from just `find-file`. > Or I can right away type a unique file name, press TAB and see it completed > to the full file name. I wonder what other people think; I still haven't > managed to get off Ido, personally. Indeed, that's why that code is commented out: inserting the common prefix is a bad idea for substring completion. I think TRT is to take the above common-prefix-directory, add it to the prompt, and remove it from each completion candidate: this will keep substring completion working correctly (and working even better since it won't uselessly find matches in the common prefix) and will also clarify where the search takes place. >> +;;;###autoload >> +(defun project-search (regexp) >> +;;;###autoload >> +(defun project-query-replace (from to) > > I'm not a fan of these names. I know they kind of mirror what we already > have in the dired- namespace, but can't we do better? If you have better ideas, by all means change them. > I think it's going to be hard for a user to differentiate between > project-find-regexp and project-search. And that they might also have > a guess that the latter "searches" for a project among the opened ones. Not only I'm not wedded to those names, but of those two commands the one that I use is project-query-replace. I only added the other one "for completeness". > Should we move both commands to the multifile package and name them, for > instance, multifile-project-find-regexp (or multifile-project-search) and > multifile-project-query-replace-regexp? No opinion on that either. > Originally we thought that this kind of UI preference would be enacted using > xref-show-xrefs-function, but apparently that's not so easy to do. I don't know what such an approach would look like. Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-26 20:13 ` Stefan Monnier @ 2018-12-27 1:49 ` Dmitry Gutov 2018-12-27 14:39 ` Stefan Monnier 2018-12-27 20:33 ` [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el Juri Linkov 0 siblings, 2 replies; 103+ messages in thread From: Dmitry Gutov @ 2018-12-27 1:49 UTC (permalink / raw) To: Stefan Monnier, emacs-devel On 26.12.2018 22:13, Stefan Monnier wrote: >> That sounds like a fair concern, never thought about it. Do we want to have >> both as generic functions, though? People will have to take care to keep >> them in sync. > > Not sure what you mean by "keep them in sync". Making sure to implement them in a compatible fashion. My point is, it's probably better to leave just one if the other can (almost?) always be efficiently implemented in terms of it. > We could define it to return a *sequence*, so it can either return > a list, or a stream, or an array, .. OK, so we can turn it into a completion table simply by calling (seq-map #'identity files). Though there might be a better choice. I wonder if we could efficiently implement project-find-regexp on top of a sequence like that, i.e. pipe to Grep with little overhead. And I'm thinking about this because there can be faster ways to list all files in the project than 'find' (e.g. 'git ls-files'). But xref-collect-matches should know nothing about 'git ls-files'. And by streams you mean stream.el in ELPA, right? It mentions "IO streams" but has no examples of that. Guess it's still something to R&D. > I think TRT is to take the above common-prefix-directory, add it to the > prompt, and remove it from each completion candidate: this will keep > substring completion working correctly (and working even better since > it won't uselessly find matches in the common prefix) and will also > clarify where the search takes place. Makes sense. Should work as long as '/' is in completion-pcm-word-delimiters. But this approach also needs the completion table to be flat, right? > Not only I'm not wedded to those names, but of those two commands the > one that I use is project-query-replace. This one also needs a docstring update (note to self or whoever). >> Should we move both commands to the multifile package and name them, for >> instance, multifile-project-find-regexp (or multifile-project-search) and >> multifile-project-query-replace-regexp? > > No opinion on that either. OK, so unless somebody objects I'd like to move them to lisp/multifile.el and rename to multifile-project-find-regexp and multifile-project-query-replace-regexp. >> Originally we thought that this kind of UI preference would be enacted using >> xref-show-xrefs-function, but apparently that's not so easy to do. > > I don't know what such an approach would look like. Hmm, maybe it's not easily compatible with multifile, since this function is passed a list of hits already. Not a list of files. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-27 1:49 ` Dmitry Gutov @ 2018-12-27 14:39 ` Stefan Monnier 2018-12-28 3:45 ` Dmitry Gutov ` (2 more replies) 2018-12-27 20:33 ` [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el Juri Linkov 1 sibling, 3 replies; 103+ messages in thread From: Stefan Monnier @ 2018-12-27 14:39 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel >> Not sure what you mean by "keep them in sync". > Making sure to implement them in a compatible fashion. My point is, it's > probably better to leave just one if the other can (almost?) always be > efficiently implemented in terms of it. Right. But I'm not sure which one should be the "canonical" one. Currently, the "canonical" one is the completion-table, and the files-list is defined based on it (while it's current definition only handles flat completion, that could be improved). > I wonder if we could efficiently implement project-find-regexp on top of > a sequence like that, i.e. pipe to Grep with little overhead. I can't see any reason why not. > And by streams you mean stream.el in ELPA, right? Indeed. Also known as lazy lists. >> I think TRT is to take the above common-prefix-directory, add it to the >> prompt, and remove it from each completion candidate: this will keep >> substring completion working correctly (and working even better since >> it won't uselessly find matches in the common prefix) and will also >> clarify where the search takes place. > > Makes sense. Should work as long as '/' is in > completion-pcm-word-delimiters. > > But this approach also needs the completion table to be flat, right? Not necessarily: we could accumulate the prefix as long as it's the sole completion. Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-27 14:39 ` Stefan Monnier @ 2018-12-28 3:45 ` Dmitry Gutov 2018-12-31 11:42 ` Dmitry Gutov 2018-12-29 0:27 ` Dmitry Gutov 2019-01-12 1:10 ` Making project-files the "canonical" generic, was: " Dmitry Gutov 2 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2018-12-28 3:45 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 27.12.2018 16:39, Stefan Monnier wrote: > Right. But I'm not sure which one should be the "canonical" one. > Currently, the "canonical" one is the completion-table, and the > files-list is defined based on it (while it's current definition only > handles flat completion, that could be improved). A flat list is a simpler structure, so we'd choose it by default. Less margin of error for implementors. What are the main benefits of making the completion table overridable? The potential of an external process doing the matching for us (and doing it faster on nonempty inputs)? Returning completion table metadata? >> I wonder if we could efficiently implement project-find-regexp on top of >> a sequence like that, i.e. pipe to Grep with little overhead. > > I can't see any reason why not. I gave it a try, to reimplement project-find-regexp based on project-files is a straightforward way, pushed to branch scratch/project-files-pipe-grep. Unfortunately, I couldn't exactly reach the performance of the find+grep version. Try: (benchmark 10 '(project-find-regexp "xyz1")) => 5.60s (benchmark 10 '(project-files-pipe-grep "xyz1")) => 6.31s or 6.10s, depending on the version Any suggestions? This is without streams, but I'm now more skeptical about their possible performance benefits in this scenario. >> But this approach also needs the completion table to be flat, right? > > Not necessarily: we could accumulate the prefix as long as it's the > sole completion. I think I see what you mean. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-28 3:45 ` Dmitry Gutov @ 2018-12-31 11:42 ` Dmitry Gutov 2018-12-31 15:12 ` Eli Zaretskii ` (2 more replies) 0 siblings, 3 replies; 103+ messages in thread From: Dmitry Gutov @ 2018-12-31 11:42 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, emacs-devel On 28.12.2018 6:45, Dmitry Gutov wrote: > I gave it a try, to reimplement project-find-regexp based on > project-files is a straightforward way, pushed to branch > scratch/project-files-pipe-grep. > > Unfortunately, I couldn't exactly reach the performance of the find+grep > version. Try: > > (benchmark 10 '(project-find-regexp "xyz1")) > => 5.60s > (benchmark 10 '(project-files-pipe-grep "xyz1")) > => 6.31s or 6.10s, depending on the version > > Any suggestions? Anybody? IMO, having this work without performance regression (or as little as possible) is a prerequisite to using 'git ls-files' in the vc project backend. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-31 11:42 ` Dmitry Gutov @ 2018-12-31 15:12 ` Eli Zaretskii 2019-01-02 23:47 ` Dmitry Gutov 2019-01-02 1:49 ` Stefan Monnier 2019-01-02 21:53 ` Juri Linkov 2 siblings, 1 reply; 103+ messages in thread From: Eli Zaretskii @ 2018-12-31 15:12 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel, monnier, juri > From: Dmitry Gutov <dgutov@yandex.ru> > Date: Mon, 31 Dec 2018 14:42:52 +0300 > Cc: Juri Linkov <juri@linkov.net>, emacs-devel@gnu.org > > On 28.12.2018 6:45, Dmitry Gutov wrote: > > Any suggestions? > > Anybody? Based on my experience, you expected responses too soon, especially given we are in holiday season. Suggest to wait for at least one more week. (Not that I necessarily see a reason for worries given the relatively small difference in timing.) Thanks. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-31 15:12 ` Eli Zaretskii @ 2019-01-02 23:47 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-02 23:47 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel, monnier, juri On 31.12.2018 18:12, Eli Zaretskii wrote: > Based on my experience, you expected responses too soon, especially > given we are in holiday season. Suggest to wait for at least one more > week. Sure. I just wanted to bring attention to that problem, since the discussion seemed to have run away in other directions. > (Not that I necessarily see a reason for worries given the relatively > small difference in timing.) Some performance testing on other machines would be helpful as well. My laptop is pretty modern. Juri reported very different, more worrying timings, and I can't explain the results. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-31 11:42 ` Dmitry Gutov 2018-12-31 15:12 ` Eli Zaretskii @ 2019-01-02 1:49 ` Stefan Monnier 2019-01-03 0:41 ` Dmitry Gutov 2019-01-02 21:53 ` Juri Linkov 2 siblings, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-01-02 1:49 UTC (permalink / raw) To: emacs-devel >> Unfortunately, I couldn't exactly reach the performance of the find+grep >> version. Try: >> >> (benchmark 10 '(project-find-regexp "xyz1")) >> => 5.60s >> (benchmark 10 '(project-files-pipe-grep "xyz1")) >> => 6.31s or 6.10s, depending on the version >> >> Any suggestions? > > Anybody? No idea, sorry. What do the 2 profiles look like? Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-02 1:49 ` Stefan Monnier @ 2019-01-03 0:41 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-03 0:41 UTC (permalink / raw) To: Stefan Monnier, emacs-devel On 02.01.2019 4:49, Stefan Monnier wrote: > No idea, sorry. What do the 2 profiles look like? project-find-regexp: - benchmark-elapse 118 36% - let 118 36% - dotimes 118 36% - let 118 36% - while 118 36% - let 118 36% - project-find-regexp 118 36% - let* 118 36% - project--find-regexp-in 94 29% - let* 82 25% - cl-mapcan 58 17% - mapcan 58 17% - #<lambda 0x52c9c08662ddee3> 58 17% - xref-collect-matches 58 17% - let* 38 11% - xref--rgrep-command 27 8% - grep-expand-template 20 6% - xref--find-ignores-arguments 16 - if 16 4% - progn 16 4% - concat 16 4% - mapconcat 16 4% - #<lambda 0x18e11839a266d1eb> - shell-quote-argument 16 4 - replace-regexp-in-string 1 apply 4 1% + eval 4 1% + require 7 2% + let 11 3% + project-ignores 12 3% + require 4 1% - xref--show-xrefs 24 7% - cond 24 7% - funcall 24 7% - xref--show-xref-buffer 24 7% - let 24 7% + save-current-buffer 24 7% + require 12 3% + project-current 24 7% project-files-pipe-grep, the latest version: - benchmark-elapse 235 11% - let 235 11% - dotimes 235 11% - let 235 11% - while 235 11% - let 235 11% - project-files-pipe-grep 235 11% - let* 235 11% - let 118 5% - let* 118 5% - cons 113 5% - cons 113 5% - cons 113 5% - append 113 5% - list 113 5% - - 109 5% - benchmark-elapse 109 5% ^ (the output is weird here) - let 109 5% + dotimes 109 5% - shell-quote-argument 4 0% - xref--regexp-to-extended 4 0% replace-regexp-in-string 4 0% - progn 5 0% - save-current-buffer 5 0% - if 5 0% - and 5 0% not 5 0% - project-files 117 5% + progn 94 4% + project-current 23 1% project-files-pipe-grep, the version with temporary file: - benchmark-elapse 140 6% - let 140 6% - dotimes 140 6% - let 140 6% - while 140 6% - let 140 6% - project-files-pipe-grep 140 6% - let* 140 6% - project-files 88 3% - progn 83 3% - all-completions 83 3% - project-file-completion-table 83 3% - progn 83 3% - progn 83 3% - let 83 3% - cl-mapcan 83 3% - mapcan 83 3% - #<lambda 0xd6ae159f08a7db4> 83 3% - let 83 3% + format 45 2% + split-string 34 1% + project-current 5 0% - let 52 2% - let* 52 2% - progn 52 2% + xref--show-xrefs 29 1% + let 19 0% + save-current-buffer 4 0% Not sure what we could read from any of this. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-31 11:42 ` Dmitry Gutov 2018-12-31 15:12 ` Eli Zaretskii 2019-01-02 1:49 ` Stefan Monnier @ 2019-01-02 21:53 ` Juri Linkov 2019-01-02 23:02 ` Dmitry Gutov 2019-01-18 3:52 ` Dmitry Gutov 2 siblings, 2 replies; 103+ messages in thread From: Juri Linkov @ 2019-01-02 21:53 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel > (benchmark 10 '(project-find-regexp "xyz1")) > => 5.60s > (benchmark 10 '(project-files-pipe-grep "xyz1")) > => 6.31s or 6.10s, depending on the version > > Any suggestions? On the very old computer from the year 2010, but the most interesting are relative times: (benchmark 10 '(project-find-regexp "xyz1")) => 7s (benchmark 10 '(project-files-pipe-grep "xyz1")) => 17s (benchmark 10 '(project-files (project-current t))) => 11s (benchmark 10 '(shell-command-to-string "find ... \\( -path ... \\) -prune -o -type f -print0")) => 11s (benchmark 10 '(shell-command-to-string "git ls-files")) => 0.07s IMHO, everything is clear: “find” with “-path” filters is slow, whereas “git ls-files” is fast. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-02 21:53 ` Juri Linkov @ 2019-01-02 23:02 ` Dmitry Gutov 2019-01-03 0:37 ` Juri Linkov 2019-01-18 3:52 ` Dmitry Gutov 1 sibling, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-02 23:02 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel On 03.01.2019 0:53, Juri Linkov wrote: > On the very old computer from the year 2010, but the most interesting are relative times: > > (benchmark 10 '(project-find-regexp "xyz1")) > => 7s > (benchmark 10 '(project-files-pipe-grep "xyz1")) > => 17s This is too bad. But did you use project-files-pipe-grep from 446bcaed37b66ec112aaec7a7960e20b969c8012 or from c708231803712bd37154c140afdfd8468cac603e? It would be helpful to test both implementations. > (benchmark 10 '(project-files (project-current t))) > => 11s This is weird. I can understand that listing all files can be slower on an old, HDD-based computer. But both project-find-regexp and project-files use 'find ... -path], and the former even adds Grep on top of it. Why is the "simpler" operation slower? Is it about piping the long list of files to Emacs? Why is the 'git ls-files' example so fast, then? It returns the same long list. > (benchmark 10 '(shell-command-to-string "find ... \\( -path ... \\) -prune -o -type f -print0")) > => 11s > > (benchmark 10 '(shell-command-to-string "git ls-files")) > => 0.07s Could you try making a full project-files implementation on top of it? I wonder how much slower it will be. At least test (split-string (shell-command-to-string "git ls-files -z") "\0" t) > IMHO, everything is clear: “find” with “-path” filters is slow, > whereas “git ls-files” is fast. We're all aware that 'git ls-files' is fast. But not every project backend is going to be using 'git ls-files' (or a Git repository). So we should make sure that project-find-regexp does not noticeably regress when using the fallback implementation of project-files (based on 'find') if we're going to change its implementation to be based on project-files. Or regresses as little as possible. And then we can implement a faster project-files for the built-in project backend (based on VC), but only, again, when used with a VCS that supports fast fetching of the list of files. And to put it in a different perspective: the difference in speed that you see between project-find-regexp and project-files-pipe-grep is from some overhead somewhere. And the same overhead is likely to manifest even if project-files is based on 'git ls-files'. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-02 23:02 ` Dmitry Gutov @ 2019-01-03 0:37 ` Juri Linkov 2019-01-03 11:45 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Juri Linkov @ 2019-01-03 0:37 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel > On 03.01.2019 0:53, Juri Linkov wrote: >> On the very old computer from the year 2010, but the most interesting are relative times: >> >> (benchmark 10 '(project-find-regexp "xyz1")) >> => 7s >> (benchmark 10 '(project-files-pipe-grep "xyz1")) >> => 17s > > This is too bad. But did you use project-files-pipe-grep from > 446bcaed37b66ec112aaec7a7960e20b969c8012 or > from c708231803712bd37154c140afdfd8468cac603e? I tried with the latest c708231803 (at least ‘vc-log-mergebase’ shows that it's the latest), but optimized: (dolist (f files) (process-send-string process f) (process-send-string process "\0")) was replaced with (process-send-string process (mapconcat #'identity files "\0")) with no performance impact. Trying 446bcaed37 has no difference either. >> (benchmark 10 '(project-files (project-current t))) >> => 11s > > This is weird. I can understand that listing all files can be slower on an > old, HDD-based computer. But both project-find-regexp and project-files use > 'find ... -path], and the former even adds Grep on top of it. Why is the > "simpler" operation slower? Are you sure that project-find-regexp uses ‘find ... -path’? I see that slowness is because of many ‘-path’ filters used in project-files. >> (benchmark 10 '(shell-command-to-string "find ... \\( -path ... \\) -prune -o -type f -print0")) >> => 11s >> >> (benchmark 10 '(shell-command-to-string "git ls-files")) >> => 0.07s > > Could you try making a full project-files implementation on top of it? > I wonder how much slower it will be. > > At least test (split-string (shell-command-to-string "git ls-files -z") "\0" t) Using this instead of ‘(project-files (project-current t))’: (benchmark 10 '(project-files-pipe-grep "xyz1")) => 3s ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-03 0:37 ` Juri Linkov @ 2019-01-03 11:45 ` Dmitry Gutov 2019-01-03 20:53 ` Juri Linkov 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-03 11:45 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel On 03.01.2019 3:37, Juri Linkov wrote: > I tried with the latest c708231803 (at least ‘vc-log-mergebase’ shows > that it's the latest), but optimized: > > (dolist (f files) > (process-send-string process f) > (process-send-string process "\0")) > > was replaced with > > (process-send-string process (mapconcat #'identity files "\0")) > > with no performance impact. I tried this modification before, and it was slower, probably because of extra memory allocation. But on your machine it's probably hard to notice. > Trying 446bcaed37 has no difference either. That's unfortunate. > Are you sure that project-find-regexp uses ‘find ... -path’? Of course. What else would it be using to get the list of files? It delegates to xref-collect-matches which, as you can see, builds a grep-find command. The salient difference is that find sends its output directly to grep, instead of going through Emacs. > I see that slowness is because of many ‘-path’ filters used in project-files.o *shrug* It is indeed slower than 'git ls-files', and especially because of the filters, but see above. >> At least test (split-string (shell-command-to-string "git ls-files -z") "\0" t) > > Using this instead of ‘(project-files (project-current t))’: > > (benchmark 10 '(project-files-pipe-grep "xyz1")) > => 3s So... I guess that's better. Have you tried running the same command in a terminal and timing it? I wonder how it compares. The terminal will show us the best possible speed. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-03 11:45 ` Dmitry Gutov @ 2019-01-03 20:53 ` Juri Linkov 2019-01-06 1:22 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Juri Linkov @ 2019-01-03 20:53 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel >>> At least test (split-string (shell-command-to-string "git ls-files -z") "\0" t) >> >> Using this instead of ‘(project-files (project-current t))’: >> >> (benchmark 10 '(project-files-pipe-grep "xyz1")) >> => 3s > > So... I guess that's better. Have you tried running the same command in > a terminal and timing it? I wonder how it compares. The terminal will show > us the best possible speed. Same numbers in a terminal. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-03 20:53 ` Juri Linkov @ 2019-01-06 1:22 ` Dmitry Gutov 2019-01-07 23:22 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-06 1:22 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel On 03.01.2019 23:53, Juri Linkov wrote: >> So... I guess that's better. Have you tried running the same command in >> a terminal and timing it? I wonder how it compares. The terminal will show >> us the best possible speed. > > Same numbers in a terminal. Curious. That doesn't shed any light on the difference in performance overhead, unfortunately. I've pushed a change that makes project-files the canonical generic and thus cuts down on some overhead, but it makes no perceptible difference on my end. Please try it out if you have the time, though. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-06 1:22 ` Dmitry Gutov @ 2019-01-07 23:22 ` Dmitry Gutov 2019-01-07 23:27 ` Dmitry Gutov ` (2 more replies) 0 siblings, 3 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-07 23:22 UTC (permalink / raw) To: Juri Linkov; +Cc: Michael Albinus, Stefan Monnier, emacs-devel On 06.01.2019 04:22, Dmitry Gutov wrote: > I've pushed a change that makes project-files the canonical generic and > thus cuts down on some overhead, but it makes no perceptible difference > on my end. Please try it out if you have the time, though. OK, I've pushed two more changes that now show better performance (a bit, but still) than project-find-regexp, at least on my machine. Testing welcome. Unfortunately, it uses call-process-region which has no Tramp counterpart. I'm really tempted to leave solving that problem up to Michael, either but adding a proper public counterpart (like a file file-process-region, with a Tramp handler), or by encapsulating the choice of whether to use call-process-region or a temporary file, in a private function. Neither seems too hard to do, though. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-07 23:22 ` Dmitry Gutov @ 2019-01-07 23:27 ` Dmitry Gutov 2019-01-08 14:21 ` Michael Albinus 2019-01-09 14:57 ` Dmitry Gutov 2 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-07 23:27 UTC (permalink / raw) To: Juri Linkov; +Cc: Michael Albinus, Stefan Monnier, emacs-devel Sorry, On 08.01.2019 02:22, Dmitry Gutov wrote: > Michael, either but adding a proper public counterpart (like a file ^ by function ^ > file-process-region, with a Tramp handler), or by encapsulating the > choice of whether to use call-process-region or a temporary file, in a > private function. Neither seems too hard to do, though. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-07 23:22 ` Dmitry Gutov 2019-01-07 23:27 ` Dmitry Gutov @ 2019-01-08 14:21 ` Michael Albinus 2019-01-08 23:06 ` Dmitry Gutov 2019-01-09 14:57 ` Dmitry Gutov 2 siblings, 1 reply; 103+ messages in thread From: Michael Albinus @ 2019-01-08 14:21 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel, Stefan Monnier, Juri Linkov Dmitry Gutov <dgutov@yandex.ru> writes: Hi Dmitry, > Unfortunately, it uses call-process-region which has no Tramp > counterpart. I'm really tempted to leave solving that problem up to > Michael, either but adding a proper public counterpart (like a file > file-process-region, with a Tramp handler), or by encapsulating the > choice of whether to use call-process-region or a temporary file, in a > private function. Neither seems too hard to do, though. Thanks for moving the ball to me :-) I believe we don't need a new file name handler function here. Writing a new function process-file-region, which uses process-file internally, should be sufficient. No Tramp expertise needed :-) Best regards, Michael. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-08 14:21 ` Michael Albinus @ 2019-01-08 23:06 ` Dmitry Gutov 2019-01-09 8:10 ` Michael Albinus 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-08 23:06 UTC (permalink / raw) To: Michael Albinus; +Cc: emacs-devel, Stefan Monnier, Juri Linkov On 08.01.2019 17:21, Michael Albinus wrote: > I believe we don't need a new file name handler function here. Writing a > new function process-file-region, which uses process-file internally, > should be sufficient. No Tramp expertise needed :-) What would it do? To clarify: I believe a certain performance improvement that I got from using call-process-region is due to avoiding a write to a temporary file (and then having the process read from it). ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-08 23:06 ` Dmitry Gutov @ 2019-01-09 8:10 ` Michael Albinus 2019-01-09 15:24 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Michael Albinus @ 2019-01-09 8:10 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel, Stefan Monnier, Juri Linkov Dmitry Gutov <dgutov@yandex.ru> writes: > On 08.01.2019 17:21, Michael Albinus wrote: > >> I believe we don't need a new file name handler function here. Writing a >> new function process-file-region, which uses process-file internally, >> should be sufficient. No Tramp expertise needed :-) > > To clarify: I believe a certain performance improvement that I got > from using call-process-region is due to avoiding a write to a > temporary file (and then having the process read from it). call-process-region uses also a temporary file. Best regards, Michael. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-09 8:10 ` Michael Albinus @ 2019-01-09 15:24 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-09 15:24 UTC (permalink / raw) To: Michael Albinus; +Cc: emacs-devel, Stefan Monnier, Juri Linkov On 09.01.2019 11:10, Michael Albinus wrote: > call-process-region uses also a temporary file. You seem to be right. Any ideas, then, why the version with call-process-region is consistently faster than the one that writes to a tmp file in Lisp? I see a stable 5% improvement. See b841ace6313a9c025038b192cc5d9efd12d93eae and 446bcaed37 in the branch scratch/project-files-pipe-grep. You can evaluate (benchmark 10 '(project-files-pipe-grep "xyz1")) in the Emacs repo to compare them. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-07 23:22 ` Dmitry Gutov 2019-01-07 23:27 ` Dmitry Gutov 2019-01-08 14:21 ` Michael Albinus @ 2019-01-09 14:57 ` Dmitry Gutov 2019-01-09 23:15 ` Juri Linkov 2 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-09 14:57 UTC (permalink / raw) To: Juri Linkov; +Cc: Michael Albinus, Stefan Monnier, emacs-devel Juri, could you please verify whether project-files-pipe-grep from b841ace6313a9c025038b192cc5d9efd12d93eae (the latest commit on the branch) is faster than the same function in 446bcaed37 (where it uses a temporary input file)? In particular, I want to understand where the the 4 extra seconds are coming from. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-09 14:57 ` Dmitry Gutov @ 2019-01-09 23:15 ` Juri Linkov 2019-01-10 10:20 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Juri Linkov @ 2019-01-09 23:15 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Michael Albinus, Stefan Monnier, emacs-devel > Juri, could you please verify whether project-files-pipe-grep from > b841ace6313a9c025038b192cc5d9efd12d93eae (the latest commit on the branch) > is faster than the same function in 446bcaed37 (where it uses a temporary > input file)? > > In particular, I want to understand where the the 4 extra seconds are > coming from. This version is better: 14s instead of previous 17s. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-09 23:15 ` Juri Linkov @ 2019-01-10 10:20 ` Dmitry Gutov 2019-01-10 21:41 ` Juri Linkov 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-10 10:20 UTC (permalink / raw) To: Juri Linkov; +Cc: Michael Albinus, Stefan Monnier, emacs-devel On 10.01.2019 02:15, Juri Linkov wrote: > This version is better: 14s instead of previous 17s. That's good, I guess. But not good enough. Let's step back. Why is project-files so slow on your machine? In particular, why is it noticeably slower than project-find-regexp? Does the latest version of project-files in the branch, over 10 invocations, still take 11s? If so, you might have to investigate more, because I'm stumped. Does it build a meaningfully different command (aside from -exec) than xref-collect-matches? Does it return a different set of files than 'git ls-files'? ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-10 10:20 ` Dmitry Gutov @ 2019-01-10 21:41 ` Juri Linkov 2019-01-12 1:48 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Juri Linkov @ 2019-01-10 21:41 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Michael Albinus, Stefan Monnier, emacs-devel >> This version is better: 14s instead of previous 17s. > > That's good, I guess. But not good enough. > > Let's step back. Why is project-files so slow on your machine? In > particular, why is it noticeably slower than project-find-regexp? Because I supposed that project-find-regexp should operate on the project's root directory, as project-files-pipe-grep and project-files do, and ran (benchmark 10 '(project-find-regexp "xyz1")) in the 'lisp' subdirectory. I remember seeing different times between running in the 'lisp' subdirectory and running in the root directory. But now can't reproduce this test case, and see the same times for project-find-regexp and project-files-pipe-grep regardless of running in the 'lisp' subdirectory or in the root directory. So it seems you already did everything possible for optimization, thank you. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-10 21:41 ` Juri Linkov @ 2019-01-12 1:48 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-12 1:48 UTC (permalink / raw) To: Juri Linkov; +Cc: Michael Albinus, Stefan Monnier, emacs-devel On 11.01.2019 00:41, Juri Linkov wrote: > I remember seeing different times between running in the 'lisp' subdirectory > and running in the root directory. I've looked at the branch history but couldn't find any reason why this might have happened, unfortunately. > But now can't reproduce this test case, > and see the same times for project-find-regexp and project-files-pipe-grep > regardless of running in the 'lisp' subdirectory or in the root directory. > So it seems you already did everything possible for optimization, thank you. Anyway, thanks for testing! I can start working on the next step now. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-02 21:53 ` Juri Linkov 2019-01-02 23:02 ` Dmitry Gutov @ 2019-01-18 3:52 ` Dmitry Gutov 2019-01-18 12:49 ` Stefan Monnier 1 sibling, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-18 3:52 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel On 03.01.2019 00:53, Juri Linkov wrote: > IMHO, everything is clear: “find” with “-path” filters is slow, > whereas “git ls-files” is fast. I've pushed the planned change. You can try this to speed things up a bit: (cl-defmethod project-files ((project (head vc)) &optional _dirs) (if (eq (ignore-errors (vc-responsible-backend default-directory)) 'Git) (let ((default-directory (vc-root-dir))) (split-string (shell-command-to-string "git ls-files -z") "\0" t)) (cl-call-next-method))) I'm not adding this to master because this does not support project-vc-ignores (or grep-find-ignored-files). Which will be the hard part. But if 'git ls-files' is enough for your needs, please enjoy the speed. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-18 3:52 ` Dmitry Gutov @ 2019-01-18 12:49 ` Stefan Monnier 2019-01-18 19:28 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-01-18 12:49 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel, Juri Linkov > I'm not adding this to master because this does not support > project-vc-ignores (or grep-find-ignored-files). Not sure why you think it should obey grep-find-ignored-files, but I think the project-vc-ignores variable is an error: project-vc-ignores should be a function that returns the set of files ignored by the underlying VCS, not something that the user sets in Emacs. So I think the code you sent which uses `git ls-files` is doing the right thing (it correctly ignores those files which project-vc-ignores should return). Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-18 12:49 ` Stefan Monnier @ 2019-01-18 19:28 ` Dmitry Gutov 2019-01-18 21:11 ` Stefan Monnier 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-18 19:28 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, emacs-devel On 18.01.2019 15:49, Stefan Monnier wrote: >> I'm not adding this to master because this does not support >> project-vc-ignores (or grep-find-ignored-files). > > Not sure why you think it should obey grep-find-ignored-files, but You're probably right, .gitignore should already include the necessary entries anyway. I'll simplify the definition of project-ignores for the vc backend. > I think the project-vc-ignores variable is an error: project-vc-ignores > should be a function that returns the set of files ignored by the > underlying VCS, not something that the user sets in Emacs. You're thinking of project-ignores. project-vc-ignores is a variable for the user to be able to tweak that list. Other editors such as Atom and VS Code have a section in the preferences for this, but The sets of files that are checked into the repository and the files that the user might like to visit or search are not necessarily the same. For instance, it's common practice where I work to check in sample config files and add the "real" file names to the ignores. Example: database.yml.sample is checked into Git, and database.yml is in gitignore (this file the user creates locally using the sample file as an example). But they'll want to visit and search that file anyway. And vice versa, there can be files inside the repository which the particular user would prefer to ignore most of the time. For me, that's usually web assets, minified JS libs, etc (I don't write JS these days anyway) and files with recorded HTTP interactions (there are lots of them, and they have very long names, so any fuzzy search against file names would often hit several of them, slowing ido-find-file down). Further, we might even choose not to use .gitignore contents in this operation and simply filter out some standard list (plus globs defined by user preference). Because if somebody decides to implement a file explorer using project-files, or only visit files using project-find-file, they might be surprised to see certain files missing. Not all ignored files are machine-generated, and even those need to be verified sometimes. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-18 19:28 ` Dmitry Gutov @ 2019-01-18 21:11 ` Stefan Monnier 2019-01-18 22:53 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-01-18 21:11 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Juri Linkov, emacs-devel > You're thinking of project-ignores. project-vc-ignores is a variable for the > user to be able to tweak that list. Other editors such as Atom and VS Code > have a section in the preferences for this, but Any reason to have this in the `vc` backend, rather than have it be a feature of project.el that applies to all projects? > The sets of files that are checked into the repository and the files that > the user might like to visit or search are not necessarily the same. You mean, we'd like to ignore ldefs-boot.el? Yeah, I see, that makes sense. Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-18 21:11 ` Stefan Monnier @ 2019-01-18 22:53 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-18 22:53 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, emacs-devel On 19.01.2019 00:11, Stefan Monnier wrote: >> You're thinking of project-ignores. project-vc-ignores is a variable for the >> user to be able to tweak that list. Other editors such as Atom and VS Code >> have a section in the preferences for this, but > > Any reason to have this in the `vc` backend, rather than have it be > a feature of project.el that applies to all projects? That's the approach I'm taking with the API: more asking, less telling. And having a variable is not the best UI there is. Third-party project backends already have some ways to configure them, so we should just let them provide the information. Projectile has .projectile which contains similar information: https://www.projectile.mx/en/latest/projects/#ignoring-files EDE has Project.ede, although it doesn't seem to include the notion of ignores. ede-ignored-file-alist contains something pertinent, though (but that variable is weird). >> The sets of files that are checked into the repository and the files that >> the user might like to visit or search are not necessarily the same. > > You mean, we'd like to ignore ldefs-boot.el? Yeah, I see, that > makes sense. More or less. Although doing that for just one file won't change much in the user experience. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-27 14:39 ` Stefan Monnier 2018-12-28 3:45 ` Dmitry Gutov @ 2018-12-29 0:27 ` Dmitry Gutov 2018-12-29 17:09 ` Dmitry Gutov 2019-01-12 1:10 ` Making project-files the "canonical" generic, was: " Dmitry Gutov 2 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2018-12-29 0:27 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 27.12.2018 16:39, Stefan Monnier wrote: >>> I think TRT is to take the above common-prefix-directory, add it to the >>> prompt, and remove it from each completion candidate: this will keep >>> substring completion working correctly (and working even better since >>> it won't uselessly find matches in the common prefix) and will also >>> clarify where the search takes place. Now done in master. But I think it illustrates a common completion pitfall we have: 1. M-x project-find-file (while inside Emacs sources) 2. Type 'GNU', then TAB, then TAB again. 3. It completes to 'GNUmakefile' and insists it is 'Sole completion'. Even though e.g. etc/GNUS-USERS exists. > Not necessarily: we could accumulate the prefix as long as it's the > sole completion. I've decided against writing a wrapper that pipes the whole collection through 'substring' every time it is called. So for now it also requires a flat collection. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-29 0:27 ` Dmitry Gutov @ 2018-12-29 17:09 ` Dmitry Gutov 2018-12-29 21:54 ` Juri Linkov 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2018-12-29 17:09 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 29.12.2018 3:27, Dmitry Gutov wrote: > But I think it illustrates a common completion pitfall we have: > > 1. M-x project-find-file (while inside Emacs sources) > 2. Type 'GNU', then TAB, then TAB again. > 3. It completes to 'GNUmakefile' and insists it is 'Sole completion'. > Even though e.g. etc/GNUS-USERS exists. Should be fixed now. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-29 17:09 ` Dmitry Gutov @ 2018-12-29 21:54 ` Juri Linkov 2018-12-30 23:06 ` Dmitry Gutov 2019-01-02 1:48 ` Stefan Monnier 0 siblings, 2 replies; 103+ messages in thread From: Juri Linkov @ 2018-12-29 21:54 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel >> But I think it illustrates a common completion pitfall we have: >> >> 1. M-x project-find-file (while inside Emacs sources) >> 2. Type 'GNU', then TAB, then TAB again. >> 3. It completes to 'GNUmakefile' and insists it is 'Sole >> completion'. Even though e.g. etc/GNUS-USERS exists. > > Should be fixed now. Thanks, now completion is much better. I noticed another problem with partial-completion - it doesn't support wildcards in the file name: 1. M-x project-find-file RET 2. Type 'm*file', then TAB, to find lisp/multifile.el 3. It completes only to the files where directory matches 'm' and file name matches 'file'. Also it seems impossible to separate words by the space character, e.g. 'm file'. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-29 21:54 ` Juri Linkov @ 2018-12-30 23:06 ` Dmitry Gutov 2019-01-02 1:48 ` Stefan Monnier 1 sibling, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2018-12-30 23:06 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel On 30.12.2018 0:54, Juri Linkov wrote: > Thanks, now completion is much better. I noticed another problem > with partial-completion - it doesn't support wildcards in the file name: > > 1. M-x project-find-file RET > 2. Type 'm*file', then TAB, to find lisp/multifile.el Seems like this would require some extra support from the completion table. Patches welcome. > 3. It completes only to the files where directory matches 'm' > and file name matches 'file'. > > Also it seems impossible to separate words by the space character, > e.g. 'm file'. I'm not sure how that input should be interpreted. Does find-file support it? ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-29 21:54 ` Juri Linkov 2018-12-30 23:06 ` Dmitry Gutov @ 2019-01-02 1:48 ` Stefan Monnier 2019-01-02 22:05 ` Juri Linkov 1 sibling, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-01-02 1:48 UTC (permalink / raw) To: emacs-devel > 2. Type 'm*file', then TAB, to find lisp/multifile.el > 3. It completes only to the files where directory matches 'm' > and file name matches 'file'. I think `m*file` will match anything that *starts* with `m` and with `file` somewhere afterwards. So it won't match `lisp/multifile.el` because that starts with `l` rather than with `m`. OTOH, I think `*m*file` should match `lisp/multifile.el`. Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-02 1:48 ` Stefan Monnier @ 2019-01-02 22:05 ` Juri Linkov 2019-01-03 3:44 ` Stefan Monnier 0 siblings, 1 reply; 103+ messages in thread From: Juri Linkov @ 2019-01-02 22:05 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel >> 2. Type 'm*file', then TAB, to find lisp/multifile.el >> 3. It completes only to the files where directory matches 'm' >> and file name matches 'file'. > > I think `m*file` will match anything that *starts* with `m` and with `file` > somewhere afterwards. So it won't match `lisp/multifile.el` because > that starts with `l` rather than with `m`. > OTOH, I think `*m*file` should match `lisp/multifile.el`. I confirm that `*m*file` matches `lisp/multifile.el`, but it's too cumbersome to type. Maybe better to use `substring` completion style? ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-02 22:05 ` Juri Linkov @ 2019-01-03 3:44 ` Stefan Monnier 2019-01-03 20:45 ` Juri Linkov 0 siblings, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-01-03 3:44 UTC (permalink / raw) To: Juri Linkov; +Cc: emacs-devel > I confirm that `*m*file` matches `lisp/multifile.el`, but it's too > cumbersome to type. Maybe better to use `substring` completion style? We could add a new style which combines substring and partial-completion, or, to put it another way, a completion style like partial-completion but without anchoring the match at the beginning. It should be very simple to implement. Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-03 3:44 ` Stefan Monnier @ 2019-01-03 20:45 ` Juri Linkov 0 siblings, 0 replies; 103+ messages in thread From: Juri Linkov @ 2019-01-03 20:45 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel >> I confirm that `*m*file` matches `lisp/multifile.el`, but it's too >> cumbersome to type. Maybe better to use `substring` completion style? > > We could add a new style which combines substring and partial-completion, > or, to put it another way, a completion style like partial-completion > but without anchoring the match at the beginning. It should be very > simple to implement. A completion style like this would be also useful in another new command where completions are displayed as file names sorted alphabetically like file-a <sub/dir1> file-a <sub/dir2> file-b <sub/dir1> Then another completion style should not match postfix directory names. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Making project-files the "canonical" generic, was: Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-27 14:39 ` Stefan Monnier 2018-12-28 3:45 ` Dmitry Gutov 2018-12-29 0:27 ` Dmitry Gutov @ 2019-01-12 1:10 ` Dmitry Gutov 2019-01-12 18:53 ` Making project-files the "canonical" generic Stephen Leake 2 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-12 1:10 UTC (permalink / raw) To: Stephen Leake; +Cc: Stefan Monnier, emacs-devel Stephen, hi! Since it was your initiative that eventually brought us project-find-file and project-file-completion-table, I'd like to ask: Did you end up ever using it and/or integrating it with ada-mode, or in some other third-party code? If so, do you see any particular benefits in keeping project-file-completion-table a generic function instead of reimplementing it on top of the (somewhat) newly added project-files generic? On 27.12.2018 17:39, Stefan Monnier wrote: >>> Not sure what you mean by "keep them in sync". >> Making sure to implement them in a compatible fashion. My point is, it's >> probably better to leave just one if the other can (almost?) always be >> efficiently implemented in terms of it. > Right. But I'm not sure which one should be the "canonical" one. > Currently, the "canonical" one is the completion-table, and the > files-list is defined based on it (while it's current definition only > handles flat completion, that could be improved). ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: Making project-files the "canonical" generic 2019-01-12 1:10 ` Making project-files the "canonical" generic, was: " Dmitry Gutov @ 2019-01-12 18:53 ` Stephen Leake 2019-01-13 0:54 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Stephen Leake @ 2019-01-12 18:53 UTC (permalink / raw) To: emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > Stephen, hi! Hi. > Since it was your initiative that eventually brought us > project-find-file and project-file-completion-table, I'd like to ask: > > Did you end up ever using it and/or integrating it with ada-mode, Yes, but not yet in the ELPA version; I had been maintaining Emacs 24 compatibility there. That's now dropped, so I can use Emacs 25 functions in future ada-mode releases. > If so, do you see any particular benefits in keeping > project-file-completion-table a generic function instead of > reimplementing it on top of the (somewhat) newly added project-files > generic? It seems to me a completion table is more useful than a simple file list; what is the use case for project-files? I've been vaguely following the multifile.el discussion, but I don't remember seeing this. I don't have a use-case for project-files, so I'd rather keep the status quo. I don't understand the comment about the default implementation of project-files only working if the completion table is "flat". Typically I use "flat" to mean "no recursive directories" when describing a path (= list of directories). I don't know what "flat" means for a completion table; what would a non-flat table be? Hmm. Maybe a "non-flat" file completion table means "return files in a directory only if the directory is completely in the string to be completed"? Do we have any examples of that? find-file works that way, but it's not a completion table. Requiring project-file-completion-table to be implemented on top of project-files prevents such non-flat completion tables, and also lazy completion tables (the ada-mode completion table is flat and lazy). Allowing user choice in completion tables is important. It seems to me that the default implementation of project-files should _not_ use project-file-completion-table, because it could easily be incorrect (it should use "find" directly). It is up to the project backend to ensure the two functions return consistent results. It would make sense to make the DIRS arg optional in project-file-completion-table, as it is in project-files. The docstring on project-files should state whether the file-names are absolute, or relative to one of DIRS, or just the basename. project-file-completion-table should state the same. Although perhaps "filename completion table" implies "absolute file names"? -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: Making project-files the "canonical" generic 2019-01-12 18:53 ` Making project-files the "canonical" generic Stephen Leake @ 2019-01-13 0:54 ` Dmitry Gutov 2019-01-15 1:14 ` Stephen Leake 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-13 0:54 UTC (permalink / raw) To: Stephen Leake, emacs-devel On 12.01.2019 21:53, Stephen Leake wrote: >> Did you end up ever using it and/or integrating it with ada-mode, > > Yes, but not yet in the ELPA version; I had been maintaining Emacs 24 > compatibility there. That's now dropped, so I can use Emacs 25 functions > in future ada-mode releases. Very good. If you don't mind sharing a link to the development repo, that would be great. >> If so, do you see any particular benefits in keeping >> project-file-completion-table a generic function instead of >> reimplementing it on top of the (somewhat) newly added project-files >> generic? > > It seems to me a completion table is more useful than a simple file > list; what is the use case for project-files? I've been vaguely > following the multifile.el discussion, but I don't remember seeing this. What is your use for completion table? I mean, the ability to provide your implementation. The default implementation should already work if project-roots and project-ignores return the correct values. The only big reason to do that that I know of, is to use a cached list of files (or somehow fetch it quickly another way, without 'find'). But project-files is just as suitable for that. > I don't have a use-case for project-files, so I'd rather keep the status > quo. project-files is now used for project-find-file, and will be used for project-find-regexp as well. The thing about it is, project-files is easier to implement. So if we don't have a particular reason for the backend author to provide their own completion table entirely, we don't need to allow that. > I don't understand the comment about the default implementation of > project-files only working if the completion table is "flat". Typically > I use "flat" to mean "no recursive directories" when describing a path > (= list of directories). I don't know what "flat" means for a completion > table; what would a non-flat table be? One where (all-completions "" table) would return not the full list of files, but e.g. include some directories without their contents directly. This can also work for completing-read if the table defines the "completion boundaries" logic. Those completion tables are fairly rare, naturally. > Hmm. Maybe a "non-flat" file completion table means "return files in a > directory only if the directory is completely in the string to be > completed"? Pretty much, I guess. > Do we have any examples of that? find-file works > that way, but it's not a completion table. ecomplete-completion-table is the one example that I found quickly, but there are probably more. > Requiring project-file-completion-table to be implemented on top of > project-files prevents such non-flat completion tables, and also lazy > completion tables (the ada-mode completion table is flat and lazy). > Allowing user choice in completion tables is important. Could you clarify about laziness? Having the list of files be returned from a method makes it "lazy" in a way already, since nobody has to fetch that list right away, they can just pass the project instance around (and call the method when necessary). > It seems to me that the default implementation of project-files should > _not_ use project-file-completion-table, because it could easily be > incorrect (it should use "find" directly). It is up to the project > backend to ensure the two functions return consistent results. If possible, I'd like to avoid this completion, and only have one of the other. After all, defining project-files on top of non-flat completion tables is also possible (I'm told), if not particularly easy. > It would make sense to make the DIRS arg optional in > project-file-completion-table, as it is in project-files. *Shrug* I have no strong opinion on that. Except that project-file-completion-table doesn't have to be a generic, as far as I can see. > The docstring on project-files should state whether the file-names are > absolute, or relative to one of DIRS, or just the basename. > project-file-completion-table should state the same. Although perhaps > "filename completion table" implies "absolute file names"? It probably does, but sure, we could use more expressive docstrings. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: Making project-files the "canonical" generic 2019-01-13 0:54 ` Dmitry Gutov @ 2019-01-15 1:14 ` Stephen Leake 2019-01-16 16:38 ` Stefan Monnier ` (2 more replies) 0 siblings, 3 replies; 103+ messages in thread From: Stephen Leake @ 2019-01-15 1:14 UTC (permalink / raw) To: emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > On 12.01.2019 21:53, Stephen Leake wrote: > >>> Did you end up ever using it and/or integrating it with ada-mode, >> >> Yes, but not yet in the ELPA version; I had been maintaining Emacs 24 >> compatibility there. That's now dropped, so I can use Emacs 25 functions >> in future ada-mode releases. > > Very good. If you don't mind sharing a link to the development repo, > that would be great. My ada-mode project stuff is not in a publicly accessible repo at the moment. I guess it's time to fix that. >>> If so, do you see any particular benefits in keeping >>> project-file-completion-table a generic function instead of >>> reimplementing it on top of the (somewhat) newly added project-files >>> generic? >> >> It seems to me a completion table is more useful than a simple file >> list; what is the use case for project-files? I've been vaguely >> following the multifile.el discussion, but I don't remember seeing this. > > What is your use for completion table? I mean, the ability to provide > your implementation. The default implementation should already work if > project-roots and project-ignores return the correct values. In my case, they don't; as I have said elsewhere, the Ada compiler tools require flat paths, so the Ada project code doesn't use project-roots, project-ignores. I have code that converts between the two, but calling that for every use of project-files would be inefficient. Providing a more flexible definition of project-path would fix that problem. > The only big reason to do that that I know of, is to use a cached list > of files (or somehow fetch it quickly another way, without 'find'). > But project-files is just as suitable for that. I could override project-files to return the correct list. The completion table style I use does uniquification by appending enough parent directories to make the file names unique (same as uniquify-buffer-name-style post-forward-angle-brackets). I might be able to make that work with the list returned by project-files, but currently some of the work is done in the completion table code itself, as it is traversing the directory path. When I wrote it, I tried but did not succeed in keeping all the completion-style-dependent code out of the table. Maybe things have changed. The completion table is also lazy, in the sense that typing characters in the completion buffer interrupts computation of the table, and it is later resumed properly. I'm not sure "lazy" is the right term for this; perhaps "interruptible"? I'm not clear if that mechanism would work thru project-files. >> I don't have a use-case for project-files, so I'd rather keep the status >> quo. > > project-files is now used for project-find-file, and will be used for > project-find-regexp as well. ? not in master as of 492ab1136860ef41c33a5e35a85fb721693f892b, fetched today. > The thing about it is, project-files is easier to implement. So if we > don't have a particular reason for the backend author to provide their > own completion table entirely, we don't need to allow that. Yes, that makes sense. >> Do we have any examples of that? find-file works >> that way, but it's not a completion table. > > ecomplete-completion-table is the one example that I found quickly, > but there are probably more. ok, so there are real examples that require project-files and project-file-completion-table to be independent of each other. And the user interface experience for completion tables is not fully determined by completion-styles-alist, so my uniquifying completion table above is not really an outlier. >> Requiring project-file-completion-table to be implemented on top of >> project-files prevents such non-flat completion tables, and also lazy >> completion tables (the ada-mode completion table is flat and lazy). >> Allowing user choice in completion tables is important. > > Could you clarify about laziness? See above. >> It seems to me that the default implementation of project-files should >> _not_ use project-file-completion-table, because it could easily be >> incorrect (it should use "find" directly). It is up to the project >> backend to ensure the two functions return consistent results. > > If possible, I'd like to avoid this completion, and only have one of > the other. After all, defining project-files on top of non-flat > completion tables is also possible (I'm told), if not particularly > easy. But defining a non-flat table on top of project-files is not possible, or at least very inefficient (you'd have to throw away all the nested directory content). So I don't think your goal is possible. Low level facilities like project.el should enable _all_ possible use cases, not rule out some in the name of simplicity. I have no objection to the default implemention of project-file-completion-table using project-files, but I don't think we should eliminate the generic function. Completion tables are a powerful feature of Emacs; we should not restrict their use with projects. -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: Making project-files the "canonical" generic 2019-01-15 1:14 ` Stephen Leake @ 2019-01-16 16:38 ` Stefan Monnier 2019-01-17 2:23 ` Dmitry Gutov 2019-01-16 19:02 ` project--completing-read-strict breaks ada-mode project completion table Stephen Leake 2019-01-17 3:04 ` Making project-files the "canonical" generic Dmitry Gutov 2 siblings, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-01-16 16:38 UTC (permalink / raw) To: emacs-devel BTW, we could also make both "canonical", i.e. allow any project system to provide either of them, and have the default implementation of both delegate to the other. Stefan diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el index 844744bf95..f971c85a7c 100644 --- a/lisp/progmodes/project.el +++ b/lisp/progmodes/project.el @@ -157,6 +157,8 @@ vc-directory-exclusion-list) grep-find-ignored-files)) +(defvar project--from-files nil) + (cl-defgeneric project-file-completion-table (project dirs) "Return a completion table for files in directories DIRS in PROJECT. DIRS is a list of absolute directories; it should be some @@ -167,6 +167,8 @@ ;; FIXME: Uniquely abbreviate the roots? (require 'xref) (let ((all-files + (if (not project--from-files) + (project-files project dirs) ;; Delegate to project--files. (cl-mapcan (lambda (dir) (let ((command @@ -178,7 +180,7 @@ (project-ignores project dir) (expand-file-name dir))))) (split-string (shell-command-to-string command) "\0" t))) - dirs))) + dirs)))) (lambda (string pred action) (cond ((eq action 'metadata) @@ -197,9 +197,10 @@ ;; returns a "flat" completion table. ;; FIXME: Maybe we should do the reverse: implement the default ;; `project-file-completion-table' on top of `project-files'. + (unless dirs (setq dirs (project-roots project))) (all-completions - "" (project-file-completion-table - project (or dirs (project-roots project))))) + "" (let ((project--from-files t)) + (project-file-completion-table project dirs)))) (defgroup project-vc nil "Project implementation using the VC package." ^ permalink raw reply related [flat|nested] 103+ messages in thread
* Re: Making project-files the "canonical" generic 2019-01-16 16:38 ` Stefan Monnier @ 2019-01-17 2:23 ` Dmitry Gutov 2019-01-17 13:25 ` Stefan Monnier 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-17 2:23 UTC (permalink / raw) To: Stefan Monnier, emacs-devel On 16.01.2019 19:38, Stefan Monnier wrote: > BTW, we could also make both "canonical", i.e. allow any project system > to provide either of them, and have the default implementation of both > delegate to the other. Sure, it's possible. But again, why allow it? So far we've only seen an example of it being used for a completion table bringing a whole new presentation with it, which is not something I would like to encourage. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: Making project-files the "canonical" generic 2019-01-17 2:23 ` Dmitry Gutov @ 2019-01-17 13:25 ` Stefan Monnier 2019-01-18 1:00 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-01-17 13:25 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel > But again, why allow it? For example because of backward compatibility. Or because there is one example of its possible use. Why disallow it? Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: Making project-files the "canonical" generic 2019-01-17 13:25 ` Stefan Monnier @ 2019-01-18 1:00 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-18 1:00 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 17.01.2019 16:25, Stefan Monnier wrote: >> But again, why allow it? > > For example because of backward compatibility. We still consider project.el "experimental", and we should be allowed to roll back on bad ideas. > Or because there is one example of its possible use. The sole goal of that example is to change the UI of project-find-file for its backend, and for that backend only. That doesn't seem like a worthy goal. And right now project-find-file is incompatible with that completion table. The obvious move it to make it use project-files instead, and thus project-file-completion-table entirely unused. Another fix could be to (try to?) move the common-parent-directory logic into the default completion table. Not sure if it's feasible, but you could prove me wrong. In that case, though, different project backends will be able to show different UIs to the users. > Why disallow it? Per-backend, non-user-customizable UIs are bad. And we've seen no other use cases so far. ^ permalink raw reply [flat|nested] 103+ messages in thread
* project--completing-read-strict breaks ada-mode project completion table 2019-01-15 1:14 ` Stephen Leake 2019-01-16 16:38 ` Stefan Monnier @ 2019-01-16 19:02 ` Stephen Leake 2019-01-16 22:02 ` Stephen Leake 2019-01-17 2:21 ` Dmitry Gutov 2019-01-17 3:04 ` Making project-files the "canonical" generic Dmitry Gutov 2 siblings, 2 replies; 103+ messages in thread From: Stephen Leake @ 2019-01-16 19:02 UTC (permalink / raw) To: emacs-devel Commit 8f9d93f3054 (Dmitry Gutov 2018-12-29 415) breaks my ada-mode completion table. That commit modifies the completion table by first calling the actual completion table to get all the file names, then removing any common prefix directory, then calling completing read with that file list. This violates assumptions made within the ada-mode completion table, and breaks the expected completion style - it requires typing the remaining directory names first; the ada-mode completion table allows completing on the base name only. I can understand the motivation behind removing common prefixes, but the proper way to do that is to implement a completion style and/or table that does that, and make that style/table the default for projects. The ada-mode completion style and table eliminates all directories from the visible completion string, except the minimum needed to make the string unique. I think this is a better solution to the problem; project.el should allow the user to choose their prefered solution, by choosing a completion style and table. To make my source available for experimenting with, I'd like to create several new ELPA packages; I sent a separate email about that. -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-16 19:02 ` project--completing-read-strict breaks ada-mode project completion table Stephen Leake @ 2019-01-16 22:02 ` Stephen Leake 2019-01-17 23:17 ` Stephen Leake 2019-01-17 2:21 ` Dmitry Gutov 1 sibling, 1 reply; 103+ messages in thread From: Stephen Leake @ 2019-01-16 22:02 UTC (permalink / raw) To: emacs-devel Stephen Leake <stephen_leake@stephe-leake.org> writes: > Commit 8f9d93f3054 (Dmitry Gutov 2018-12-29 415) breaks my ada-mode > completion table. > > That commit modifies the completion table by first calling the actual > completion table to get all the file names, then removing any common > prefix directory, then calling completing read with that file list. > > This violates assumptions made within the ada-mode completion table, and > breaks the expected completion style - it requires typing the remaining > directory names first; the ada-mode completion table allows completing > on the base name only. > > I can understand the motivation behind removing common prefixes, but the > proper way to do that is to implement a completion style and/or table > that does that, and make that style/table the default for projects. > > The ada-mode completion style and table eliminates all directories from > the visible completion string, except the minimum needed to make the > string unique. I think this is a better solution to the problem; > project.el should allow the user to choose their prefered solution, by > choosing a completion style and table. > > To make my source available for experimenting with, I'd like to create > several new ELPA packages; I sent a separate email about that. These are now in the ELPA master branch: elpa/packages/ada-mode/ada-project.el elpa/packages/ada-mode/env-package.el elpa/packages/path-iterator/path-iterator.el elpa/packages/uniquify-files/uniquify-files.el -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-16 22:02 ` Stephen Leake @ 2019-01-17 23:17 ` Stephen Leake 2019-01-18 2:04 ` Dmitry Gutov 2019-01-20 19:34 ` Stephen Leake 0 siblings, 2 replies; 103+ messages in thread From: Stephen Leake @ 2019-01-17 23:17 UTC (permalink / raw) To: emacs-devel [-- Attachment #1: Type: text/plain, Size: 2182 bytes --] Stephen Leake <stephen_leake@stephe-leake.org> writes: > These are now in the ELPA master branch: > > elpa/packages/ada-mode/ada-project.el > elpa/packages/ada-mode/env-package.el > elpa/packages/path-iterator/path-iterator.el > elpa/packages/uniquify-files/uniquify-files.el Attached is an example project for emacs source to play with. To use it, eval the following in emacs master *scratch* (adjust paths); ---------------- (add-to-list 'load-path "c:/Projects/elpa/packages/uniquify-files") (add-to-list 'load-path "c:/Projects/elpa/packages/path-iterator") (load-file "c:/Projects/emacs_stephe.main/projects/emacs-project.el") ;; revert to emacs-26 version of project--completing-read-strict ;; (skip this to show the problem with the master version) (defun project--completing-read-strict (prompt collection &optional predicate hist default inherit-input-method) ;; Tried both expanding the default before showing the prompt, and ;; removing it when it has no matches. Neither seems natural ;; enough. Removal is confusing; early expansion makes the prompt ;; too long. (let* ((new-prompt (if default (format "%s (default %s): " prompt default) (format "%s: " prompt))) (res (completing-read new-prompt collection predicate t nil hist default inherit-input-method))) (if (and (equal res default) (not (test-completion res collection predicate))) (completing-read (format "%s: " prompt) collection predicate t res hist nil inherit-input-method) res))) (find-file "c:/Projects/emacs/master/lisp/files.el") ----------------- Then type "M-x project-find-file", followed by "proje" <tab> <tab>. Or use icomplete-mode, use C-. , C-, to rotate completions. There's a bug in path-iterator; I've set :ignore-function to ignore any dir under ".git", but they are showing up anyway. But that demonstrates how non-unique file names are handled, so it's good for this demo. -- -- Stephe [-- Attachment #2: emacs-project.el --] [-- Type: application/emacs-lisp, Size: 1130 bytes --] ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-17 23:17 ` Stephen Leake @ 2019-01-18 2:04 ` Dmitry Gutov 2019-01-19 3:35 ` Stephen Leake 2019-01-20 19:34 ` Stephen Leake 1 sibling, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-18 2:04 UTC (permalink / raw) To: Stephen Leake, emacs-devel On 18.01.2019 02:17, Stephen Leake wrote: > Stephen Leake <stephen_leake@stephe-leake.org> writes: > >> These are now in the ELPA master branch: >> >> elpa/packages/ada-mode/ada-project.el >> elpa/packages/ada-mode/env-package.el >> elpa/packages/path-iterator/path-iterator.el >> elpa/packages/uniquify-files/uniquify-files.el > > Attached is an example project for emacs source to play with. To use it, > eval the following in emacs master *scratch* (adjust paths); I've tried it out, and it's pretty nice. The latency seemed to be a bit bigger than with the default completion table based on 'find', but not significantly so. path-iterator doesn't have to handle the contents of .gitignore, so it's not a huge surprise. If support is added, it's sure to become slower. I don't think it can be *the* UI for everybody. Yeah, being able to type abbreviated names can be nice, but a lot of users will prefer full directories and fuzzy matching based on completion styles. Segregating by project types probably won't work well either. > There's a bug in path-iterator; I've set :ignore-function to ignore any > dir under ".git", but they are showing up anyway. The first thing I've noticed, actually. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-18 2:04 ` Dmitry Gutov @ 2019-01-19 3:35 ` Stephen Leake 2019-01-19 22:05 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Stephen Leake @ 2019-01-19 3:35 UTC (permalink / raw) To: emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > On 18.01.2019 02:17, Stephen Leake wrote: >> Stephen Leake <stephen_leake@stephe-leake.org> writes: >> >>> These are now in the ELPA master branch: >>> >>> elpa/packages/ada-mode/ada-project.el >>> elpa/packages/ada-mode/env-package.el >>> elpa/packages/path-iterator/path-iterator.el >>> elpa/packages/uniquify-files/uniquify-files.el >> >> Attached is an example project for emacs source to play with. To use it, >> eval the following in emacs master *scratch* (adjust paths); > > I've tried it out, and it's pretty nice. Thanks. > The latency seemed to be a bit bigger than with the default completion > table based on 'find', but not significantly so. Fixing the ignore would help that; it's processing all of .git. > I don't think it can be *the* UI for everybody. Of course. And this is the main point; project.el must allow users/clients to choose the completion table and completion style. So the recent change to project--completing-read-strict should be reverted, and implemented as a completion style/table instead. -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-19 3:35 ` Stephen Leake @ 2019-01-19 22:05 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-19 22:05 UTC (permalink / raw) To: Stephen Leake, emacs-devel On 19.01.2019 06:35, Stephen Leake wrote: >> The latency seemed to be a bit bigger than with the default completion >> table based on 'find', but not significantly so. > > Fixing the ignore would help that; it's processing all of .git. It doesn't handle the contents of .gitignore, though. That would add some performance penalty by itself. >> I don't think it can be *the* UI for everybody. > > Of course. And this is the main point; project.el must allow > users/clients to choose the completion table and completion style. Users, not necessarily backends. And let's say "completion style" in the general sense. > So the recent change to project--completing-read-strict should be > reverted, and implemented as a completion style/table instead. Please see the rest of this discussion. I think it's now the consensus that the uniquify logic should move out of the completion table. We can apply it inside project-find-file itself, depending on a user option. That will require changes in project--completing-read-strict too, of course. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-17 23:17 ` Stephen Leake 2019-01-18 2:04 ` Dmitry Gutov @ 2019-01-20 19:34 ` Stephen Leake 1 sibling, 0 replies; 103+ messages in thread From: Stephen Leake @ 2019-01-20 19:34 UTC (permalink / raw) To: emacs-devel Stephen Leake <stephen_leake@stephe-leake.org> writes: > There's a bug in path-iterator; I've set :ignore-function to ignore any > dir under ".git", but they are showing up anyway. But that demonstrates > how non-unique file names are handled, so it's good for this demo. The bug was not in path-iterator but in the ignore function; args to string-match are reversed. In addition, the function can be simplified, since if dir is ignored, no children of dir will be passed to the function: (defun emacs-project-ignore-dir (dir) "Return non-nil if 'dir' should be ignored in file-completion-table." ;; If DIR is ignored, no children of DIR will be passed to this function. (string-equal ".git" (file-name-nondirectory dir))) -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-16 19:02 ` project--completing-read-strict breaks ada-mode project completion table Stephen Leake 2019-01-16 22:02 ` Stephen Leake @ 2019-01-17 2:21 ` Dmitry Gutov 2019-01-17 13:55 ` Stefan Monnier 1 sibling, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-17 2:21 UTC (permalink / raw) To: Stephen Leake, emacs-devel Hi Stephen, On 16.01.2019 22:02, Stephen Leake wrote: > Commit 8f9d93f3054 (Dmitry Gutov 2018-12-29 415) breaks my ada-mode > completion table. I'm sorry to hear that, but: > That commit modifies the completion table by first calling the actual > completion table to get all the file names, then removing any common > prefix directory, then calling completing read with that file list. Yup. It solved a valid user complaint, with a solution proposed by Stefan. > This violates assumptions made within the ada-mode completion table, and > breaks the expected completion style - it requires typing the remaining > directory names first; the ada-mode completion table allows completing > on the base name only. TBH, I kind of struggling to understand how the previous version worked for you. But I guess when a completion table comes with a strongly coupled completion style, it's kind of possible. > I can understand the motivation behind removing common prefixes, but the > proper way to do that is to implement a completion style and/or table > that does that, and make that style/table the default for projects. How exactly would you propose to do that? I don't see a way. Styles are an entirely different thing (they don't modify how completion strings are presented), and as for a table... for one thing, if a table removes a common parent directory from the strings, how would it pass that information to the command that's going to use it? To expand the file name later on. > The ada-mode completion style and table eliminates all directories from > the visible completion string, except the minimum needed to make the > string unique. I think this is a better solution to the problem; > project.el should allow the user to choose their prefered solution, by > choosing a completion style and table. The user is not choosing a table. The project backend does, and in your case it presents an entirely different look for file names from the other backends, with no way for the user to change it. I think that's bad. Namely, that e.g. M-x project-find-file can look drastically different for the same user when using different project backends. Far be it from me to criticize the exact "uniquified" look for the entries, but I don't think it's a good place to produce them. If it's not possible to implement them via a completion style or something like that, that would apply to all project backends and their completion tables if the user so chooses, maybe it should just be a different command. E.g. named project-uniquify-find-file which would modify the completion table returned by the project (or, more trivially, the list of files). Whereas the completion tables should contain more or less the same kind of entries. IOW, to me this looks like an argument *against* having project-file-completion-table be a generic. The project backend should produce *data*, not formatting, in the same uniform format, for all backends. The more differences there are, the harder it's going to be to consume. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-17 2:21 ` Dmitry Gutov @ 2019-01-17 13:55 ` Stefan Monnier 2019-01-17 21:35 ` John Yates 2019-01-18 2:19 ` Dmitry Gutov 0 siblings, 2 replies; 103+ messages in thread From: Stefan Monnier @ 2019-01-17 13:55 UTC (permalink / raw) To: emacs-devel >> I can understand the motivation behind removing common prefixes, but the >> proper way to do that is to implement a completion style and/or table >> that does that, and make that style/table the default for projects. > How exactly would you propose to do that? His completion style relies on an advice to the completing-read code which re-adds the text that was stripped during the completion. Clearly, that won't work when used for completion-at-point. I actually think his approach is interesting and maybe we should think how our completion system could provide support for these kinds of "styles". >> The ada-mode completion style and table eliminates all directories from >> the visible completion string, except the minimum needed to make the >> string unique. I think this is a better solution to the problem; >> project.el should allow the user to choose their prefered solution, by >> choosing a completion style and table. > The user is not choosing a table. The project backend does, and in your case > it presents an entirely different look for file names from the other > backends, with no way for the user to change it. > I think that's bad. Indeed, an important design constraint I followed for the completion styles is that the user should be able to choose the style (hence the indirection through completion-category-overrides). I think part of Stephen's argument is that the same comment applies to the new completion behavior of M-x project-find-file you installed in 8f9d93f3054. > Far be it from me to criticize the exact "uniquified" look for the entries, > but I don't think it's a good place to produce them. If it's not possible to > implement them via a completion style or something like that, that would > apply to all project backends and their completion tables if the user so > chooses, maybe it should just be a different command. I'm not sure if it currently can work for any project backend, but its design definitely aims for it to be backend-agnostic so it can be used for any (flat) completion table (I don't think it'll work well with completion-file-name-table). Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-17 13:55 ` Stefan Monnier @ 2019-01-17 21:35 ` John Yates 2019-01-18 2:19 ` Dmitry Gutov 1 sibling, 0 replies; 103+ messages in thread From: John Yates @ 2019-01-17 21:35 UTC (permalink / raw) To: Stefan Monnier; +Cc: Emacs developers On Thu, Jan 17, 2019 at 8:56 AM Stefan Monnier <monnier@iro.umontreal.ca> wrote: > > His completion style relies on an advice to the completing-read code > which re-adds the text that was stripped during the completion. > Clearly, that won't work when used for completion-at-point. > > I actually think his approach is interesting and maybe we should think > how our completion system could provide support for these kinds of > "styles". > > >> The ada-mode completion style and table eliminates all directories from > >> the visible completion string, except the minimum needed to make the > >> string unique. I think this is a better solution to the problem; > >> project.el should allow the user to choose their prefered solution, by > >> choosing a completion style and table. I have been only skimming this thread. The above caught my eye. The description seems very similar to my own wsf.el (for WorkSpace Files). It uses ivy as its completion framework. Not being much of an elisp programmer I have made no effort to publicize it. Still you can see what I have here: https://github.com/jsyjr/MyConfiguration/blob/master/emacs/wsf/wsf.el Of course I would be very happy to have such functionality supplied directly by emacs. /john ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-17 13:55 ` Stefan Monnier 2019-01-17 21:35 ` John Yates @ 2019-01-18 2:19 ` Dmitry Gutov 2019-01-18 3:05 ` Stefan Monnier ` (2 more replies) 1 sibling, 3 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-18 2:19 UTC (permalink / raw) To: Stefan Monnier, emacs-devel On 17.01.2019 16:55, Stefan Monnier wrote: > I actually think his approach is interesting and maybe we should think > how our completion system could provide support for these kinds of > "styles". Sure, but in some different way. Having some wrapper middleware could work, but I'm afraid it might stumble against the same problem the current project-find-file does: completion tables are too flexible. So it's hard to make a universal wrapper which would modify them all the same. > Indeed, an important design constraint I followed for the completion > styles is that the user should be able to choose the style (hence the > indirection through completion-category-overrides). Yup. > I think part of Stephen's argument is that the same comment applies to > the new completion behavior of M-x project-find-file you installed > in 8f9d93f3054. Like already said, nobody's stopping you or him or anyone else from adding a new command that shows file names in a different way (e.g. uniquified/abbreviated). As long as both project-find-file and the new command work the same across all project backends, I'm happy. And again, it doesn't have to be a separate command, the behavior could be customizable and dispatched inside the one command's implementation. We can't do that if the said behavior already lives inside the completion table, however. >> Far be it from me to criticize the exact "uniquified" look for the entries, >> but I don't think it's a good place to produce them. If it's not possible to >> implement them via a completion style or something like that, that would >> apply to all project backends and their completion tables if the user so >> chooses, maybe it should just be a different command. > > I'm not sure if it currently can work for any project backend, but its > design definitely aims for it to be backend-agnostic By "it", do you mean uniquify-files.el? > so it can be used > for any (flat) completion table (I don't think it'll work well with > completion-file-name-table). Would you agree that a "flat completion table" is basically always a list of files? ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-18 2:19 ` Dmitry Gutov @ 2019-01-18 3:05 ` Stefan Monnier 2019-01-19 0:26 ` Dmitry Gutov 2019-01-21 19:32 ` Stephen Leake 2019-01-21 19:36 ` Stephen Leake 2 siblings, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-01-18 3:05 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel > We can't do that if the said behavior already lives inside the completion > table, however. Clearly. >> I'm not sure if it currently can work for any project backend, but its >> design definitely aims for it to be backend-agnostic > By "it", do you mean uniquify-files.el? Yes. >> so it can be used for any (flat) completion table (I don't think >> it'll work well with completion-file-name-table). > Would you agree that a "flat completion table" is basically always a list > of files? In some corner cases maybe not, but in the context of uniquify-files: yes definitely. Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-18 3:05 ` Stefan Monnier @ 2019-01-19 0:26 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-19 0:26 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 18.01.2019 06:05, Stefan Monnier wrote: >>> so it can be used for any (flat) completion table (I don't think >>> it'll work well with completion-file-name-table). >> Would you agree that a "flat completion table" is basically always a list >> of files? > In some corner cases maybe not, but in the context of uniquify-files: > yes definitely. You probably see what I'm getting at. If this is the data we need, it's the data we should ask for from a project backend, and not a completion table. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-18 2:19 ` Dmitry Gutov 2019-01-18 3:05 ` Stefan Monnier @ 2019-01-21 19:32 ` Stephen Leake 2019-01-22 0:09 ` Dmitry Gutov 2019-02-07 1:20 ` Stephen Leake 2019-01-21 19:36 ` Stephen Leake 2 siblings, 2 replies; 103+ messages in thread From: Stephen Leake @ 2019-01-21 19:32 UTC (permalink / raw) To: emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > And again, it doesn't have to be a separate command, the behavior > could be customizable and dispatched inside the one command's > implementation. I'm working on this. Life is intervening, so it may take a while. -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-21 19:32 ` Stephen Leake @ 2019-01-22 0:09 ` Dmitry Gutov 2019-02-07 1:20 ` Stephen Leake 1 sibling, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-22 0:09 UTC (permalink / raw) To: Stephen Leake, emacs-devel On 21.01.2019 22:32, Stephen Leake wrote: >> And again, it doesn't have to be a separate command, the behavior >> could be customizable and dispatched inside the one command's >> implementation. > > I'm working on this. > > Life is intervening, so it may take a while. Thanks! There's no hurry, from where I'm standing. As long as we more or less agree on the approach. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-21 19:32 ` Stephen Leake 2019-01-22 0:09 ` Dmitry Gutov @ 2019-02-07 1:20 ` Stephen Leake 2019-02-11 21:50 ` Stefan Monnier 1 sibling, 1 reply; 103+ messages in thread From: Stephen Leake @ 2019-02-07 1:20 UTC (permalink / raw) To: emacs-devel [-- Attachment #1: Type: text/plain, Size: 3275 bytes --] Stephen Leake <stephen_leake@stephe-leake.org> writes: > Dmitry Gutov <dgutov@yandex.ru> writes: > >> And again, it doesn't have to be a separate command, the behavior >> could be customizable and dispatched inside the one command's >> implementation. I've now implemented this. Gnu ELPA has updated versions of path-iterator and uniquified-files. I added a new file completion style; file-root-rel, in uniquified-files/file-complete-root-relative.el. Attached is an elisp file demo-project-file-completion.el, which rewrites project--completing-read-strict to allow either the user or the project backend to specify the completion style, and provides examples of each of three styles: "default"; substring style on absolute file names uniquified-file: complete on basename, then on appended uniquifying directories. (Juri Linkov mentioned a style like this in https://lists.gnu.org/archive/html/emacs-devel/2019-01/msg00081.html) file-root-rel: complete on filenames relative to the project root (assumes a single root). Also attached are emacs-project.el, uniquify-test-project.el, providing example projects used in the demo. A few points: The file names may want to change; file-root-rel reuses some of uniquify-file, and 'uniquify-file' might be better named 'file-basename'. project-file-completion-table can return a list of files, so a backend can use 'git ls' or 'find' to implement it. That is no harder to implement than 'project-files'. The uniquify-file completion style works on a list of files; see the demo case with (uniquify-test-project-completion-style 'default) (project-file-completion-styles '(uniquify-file)) However, file-root-rel does not, because the root directory must be stored somewhere that is accessible from the completion code. project--completing-read-strict handles this by wrapping a list in a completion table function. We could use a global variable to store the root instead, but that's not much better. If there are other completion styles that have global data stored somewhere, perhaps it would make sense to introduce a root defstruct for completion styles, store the completion table and data in that object, and pass that to completion functions instead of the completion table. That could be backward compatible; completion functions would check for the defstruct as well as for a function or a list. Note that project--completing-read-strict let-binds both completion-category-overrides and completion-styles, in order to handle all the possible choices. Some completion table functions require a particular style, mostly for optimization reasons, but also for the global data issue; that is indicated by returning a "style" metadata. Some backends will want to specify the style; Ada works well with uniquify-file, since the file basenames are required to be unique. Java works well with file-root-rel, since the directory part of the basename is significant. uniquify-files.el now adds two functions to completion-styles-alist, for converting strings from user to table input format, or user to data format. It also adds a "style" metadata to the completion table function API. Together with the advice on completing-read-default and test-completion, this could be moved to minibuffer.el. -- -- Stephe [-- Attachment #2: demo-project-file-completion.el --] [-- Type: application/emacs-lisp, Size: 4526 bytes --] [-- Attachment #3: emacs-project.el --] [-- Type: application/emacs-lisp, Size: 1888 bytes --] [-- Attachment #4: uniquify-test-project.el --] [-- Type: application/emacs-lisp, Size: 1714 bytes --] ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-02-07 1:20 ` Stephen Leake @ 2019-02-11 21:50 ` Stefan Monnier 2019-02-12 1:31 ` Stephen Leake 0 siblings, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-02-11 21:50 UTC (permalink / raw) To: emacs-devel >>> And again, it doesn't have to be a separate command, the behavior >>> could be customizable and dispatched inside the one command's >>> implementation. > I've now implemented this. > Gnu ELPA has updated versions of path-iterator and uniquified-files. > I added a new file completion style; file-root-rel, in > uniquified-files/file-complete-root-relative.el. > > Attached is an elisp file demo-project-file-completion.el, which > rewrites project--completing-read-strict to allow either the user or the > project backend to specify the completion style, and provides examples > of each of three styles: [...] > (table-styles (cdr (assq 'styles (completion-metadata "" collection nil)))) > (completion-category-overrides > (list (list 'project-file (cons 'styles > (or table-styles > project-file-completion-styles))))) > > ;; If the completion table is a list, or a function that does > ;; not return styles metadata, we set completion-styles to > ;; reflect the user choice. > (completion-styles (if table-styles nil project-file-completion-styles)) This gives precedence to the collection's styles with no way for the user to override this choice. This problem is the reason why I designed the current system to go through the indirection of a category, making it possible for the user to override the category's default styles via completion-category-overrides. > The uniquify-file completion style works on a list of files; see the [...] > However, file-root-rel does not, because the root directory must be > stored somewhere that is accessible from the completion code. I don't understand this. Why can't the completion style compute the common-prefix? > uniquify-files.el now adds two functions to completion-styles-alist, for > converting strings from user to table input format, or user to data > format. As I mentioned in another message to João, I think we should move from completion-style-alist to using generic functions that dispatch on the style. Also, I don't quite understand why we need 2: they both seem to implement the same direction of the conversion (i.e. from "user-string" to "data-string"). I see that uniq-file-get-data-string does more (i.e. it tries to find the corresponding match if there's only one) but I don't understand why you need to do that: this seems to do a kind of completion which minibuffer-complete-and-exit should already have done (if `require-match` is set) or which shouldn't be done (if `require-match` is not set). > Together with the advice on completing-read-default and > test-completion, this could be moved to minibuffer.el. As you mention in uniquify-files.el: (defun completion-get-data-string (user-string table pred) [...] ;; FIXME: This is ultimately called from ;; `completion-try-completion' or `completion-all-completions'; ;; there is only one style currently being used. Need to pass that ;; style from there to here. it only makes sense to call the conversion function corresponding to the style that was used to generate that string. [ Also while a specific call to minibuffer-complete (or minibuffer-completion-help, or minibuffer-force-complete, ...) only uses a single style, a given completing-read session can currently use several completion styles. ] So I think we should fix this FIXME before we can move this code to minibuffer.el. Maybe we can save the completion style that returned that string in a text-property, or even directly store the conversion function in there (so we don't need to extend completion-style-alist). Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-02-11 21:50 ` Stefan Monnier @ 2019-02-12 1:31 ` Stephen Leake 2019-02-15 15:50 ` Stephen Leake 2019-02-19 17:45 ` Stefan Monnier 0 siblings, 2 replies; 103+ messages in thread From: Stephen Leake @ 2019-02-12 1:31 UTC (permalink / raw) To: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> (table-styles (cdr (assq 'styles (completion-metadata "" collection nil)))) >> (completion-category-overrides >> (list (list 'project-file (cons 'styles >> (or table-styles >> project-file-completion-styles))))) >> >> ;; If the completion table is a list, or a function that does >> ;; not return styles metadata, we set completion-styles to >> ;; reflect the user choice. >> (completion-styles (if table-styles nil project-file-completion-styles)) > > This gives precedence to the collection's styles with no way for the > user to override this choice. Yes, because a collection should only specify a style if it cannot work with other styles. For example, if you comment out the styles metadata in uniq-file-completion-table (line 717 in uniquify-files.el), and set (uniquify-test-project-completion-style 'uniquify-file) (project-file-completion-styles '(substring)), the completion does not work properly; you can complete on the basename, but not on the directory part. Similarly for the styles metadata in fc-root-rel-completion-table-iter. I need to come up with a concise explanation of when a completion table should return a style. In this demo, there are two user-setable variables to indicate completion style; uniquify-test-project-completion-style at the project backend level, project-file-completion-styles at the generic project level. The user can set either, so they always have a choice. That should be a general rule; if a project backend provides a completion table that requires a certain style, it should also provide a plain list so the user can choose the style. > This problem is the reason why I designed the current system to go > through the indirection of a category, making it possible for the user > to override the category's default styles via > completion-category-overrides. > >> The uniquify-file completion style works on a list of files; see the > [...] >> However, file-root-rel does not, because the root directory must be >> stored somewhere that is accessible from the completion code. > > I don't understand this. Why can't the completion style compute the > common-prefix? The root is _not_ the common prefix of the current matches; it's an arbitrary directory, nominally the single project root. In project--completing-read-strict, it's the common prefix of the entire collection, computed once at the start. If the style computed the common prefix on each match set, it might not be constant during the completion; it could get longer as the choices are narrowed. That does not happen in project--completing-read-strict in master, nor in file-root-rel. >> uniquify-files.el now adds two functions to completion-styles-alist, for >> converting strings from user to table input format, or user to data >> format. > > As I mentioned in another message to João, I think we should move from > completion-style-alist to using generic functions that dispatch on the > style. That makes sense. > Also, I don't quite understand why we need 2: they both seem to > implement the same direction of the conversion (i.e. from "user-string" > to "data-string"). No, "table input format" is not the same as "data format". See the header comments in uniquify-files; "table input" is the format required for the "string" parameter to the completion table functions; for both uniquify-file and file-root-rel it is "partial_dir/partial_basename". "data" is the format returned by all-completions; an absolute file name. > I see that uniq-file-get-data-string does more (i.e. it tries to find > the corresponding match if there's only one) but I don't understand > why you need to do that: this seems to do a kind of completion which > minibuffer-complete-and-exit should already have done (if > `require-match` is set) or which shouldn't be done (if `require-match` > is not set). The main computation in uniq-file-get-data-string is to return the absolute file name corresponding to the table input string. minibuffer-complete-and-exit can't do that, because it doesn't know how to convert the user string to the absolute file name; as far as it knows, "foo-file1.text<Alice/alice-1/>" is not a valid completion for "c:/.../uniquify-file-resources/Alice/alice-1/foo-file1.text", because they are not string-equal. Another way to look at this; the minibuffer-level completion functions only have access to user format strings; ie "foo-file1.text<Alice/alice-1/>". minibuffer-complete-and-exit could return that, but we want it to return the absolute file name, as the substring completion style does, when used on a list of absolute file names. >> Together with the advice on completing-read-default and >> test-completion, this could be moved to minibuffer.el. > > As you mention in uniquify-files.el: > > (defun completion-get-data-string (user-string table pred) > [...] > ;; FIXME: This is ultimately called from > ;; `completion-try-completion' or `completion-all-completions'; > ;; there is only one style currently being used. Need to pass that > ;; style from there to here. > > it only makes sense to call the conversion function corresponding to the > style that was used to generate that string. > > [ Also while a specific call to minibuffer-complete (or > minibuffer-completion-help, or minibuffer-force-complete, ...) only > uses a single style, a given completing-read session can currently use > several completion styles. ] > > So I think we should fix this FIXME before we can move this code to > minibuffer.el. Maybe we can save the completion style that returned > that string in a text-property, or even directly store the conversion > function in there (so we don't need to extend completion-style-alist). If we move to a completion object storing various things, we can store it there. Which doesn't address making this work in emacs 26. Hmm. This FIXME comment lies; completion-get-data-string is called after completing-read-default, via uniq-file-completing-read-default-advice. At that point, user-string was computed by the last style tried (ie uniq-file-all-completions, which could set a text property), but I'm not sure if it's copied (losing the text properties) in the middle somewhere. I'll try it. Similarly for completion-to-table-input. -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-02-12 1:31 ` Stephen Leake @ 2019-02-15 15:50 ` Stephen Leake 2019-02-15 22:47 ` Stephen Leake 2019-04-19 17:49 ` Stephen Leake 2019-02-19 17:45 ` Stefan Monnier 1 sibling, 2 replies; 103+ messages in thread From: Stephen Leake @ 2019-02-15 15:50 UTC (permalink / raw) To: emacs-devel Stephen Leake <stephen_leake@stephe-leake.org> writes: > Stefan Monnier <monnier@iro.umontreal.ca> writes: > >> >> As you mention in uniquify-files.el: >> >> (defun completion-get-data-string (user-string table pred) >> [...] >> ;; FIXME: This is ultimately called from >> ;; `completion-try-completion' or `completion-all-completions'; >> ;; there is only one style currently being used. Need to pass that >> ;; style from there to here. >> >> it only makes sense to call the conversion function corresponding to the >> style that was used to generate that string. >> >> [ Also while a specific call to minibuffer-complete (or >> minibuffer-completion-help, or minibuffer-force-complete, ...) only >> uses a single style, a given completing-read session can currently use >> several completion styles. ] >> >> So I think we should fix this FIXME before we can move this code to >> minibuffer.el. Maybe we can save the completion style that returned >> that string in a text-property, or even directly store the conversion >> function in there (so we don't need to extend completion-style-alist). > > ... completion-get-data-string is called after > completing-read-default, via uniq-file-completing-read-default-advice. > At that point, user-string was computed by the last style tried (ie > uniq-file-all-completions, which could set a text property), but I'm not > sure if it's copied (losing the text properties) in the middle > somewhere. I'll try it. This is now implemented, in ELPA. -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-02-15 15:50 ` Stephen Leake @ 2019-02-15 22:47 ` Stephen Leake 2019-02-15 23:38 ` Stephen Leake 2019-04-19 17:49 ` Stephen Leake 1 sibling, 1 reply; 103+ messages in thread From: Stephen Leake @ 2019-02-15 22:47 UTC (permalink / raw) To: emacs-devel Stephen Leake <stephen_leake@stephe-leake.org> writes: > Stephen Leake <stephen_leake@stephe-leake.org> writes: > >> Stefan Monnier <monnier@iro.umontreal.ca> writes: >> >>> >>> As you mention in uniquify-files.el: >>> >>> (defun completion-get-data-string (user-string table pred) >>> [...] >>> ;; FIXME: This is ultimately called from >>> ;; `completion-try-completion' or `completion-all-completions'; >>> ;; there is only one style currently being used. Need to pass that >>> ;; style from there to here. >>> >>> it only makes sense to call the conversion function corresponding to the >>> style that was used to generate that string. >>> >>> [ Also while a specific call to minibuffer-complete (or >>> minibuffer-completion-help, or minibuffer-force-complete, ...) only >>> uses a single style, a given completing-read session can currently use >>> several completion styles. ] >>> >>> So I think we should fix this FIXME before we can move this code to >>> minibuffer.el. Maybe we can save the completion style that returned >>> that string in a text-property, or even directly store the conversion >>> function in there (so we don't need to extend completion-style-alist). >> >> ... completion-get-data-string is called after >> completing-read-default, via uniq-file-completing-read-default-advice. >> At that point, user-string was computed by the last style tried (ie >> uniq-file-all-completions, which could set a text property), but I'm not >> sure if it's copied (losing the text properties) in the middle >> somewhere. I'll try it. > > This is now implemented, in ELPA. Except it also needs: (setq minibuffer-allow-text-properties t) -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-02-15 22:47 ` Stephen Leake @ 2019-02-15 23:38 ` Stephen Leake 0 siblings, 0 replies; 103+ messages in thread From: Stephen Leake @ 2019-02-15 23:38 UTC (permalink / raw) To: emacs-devel Stephen Leake <stephen_leake@stephe-leake.org> writes: > Stephen Leake <stephen_leake@stephe-leake.org> writes: > >> Stephen Leake <stephen_leake@stephe-leake.org> writes: >> >>> Stefan Monnier <monnier@iro.umontreal.ca> writes: >>> >>>> >>>> As you mention in uniquify-files.el: >>>> >>>> (defun completion-get-data-string (user-string table pred) >>>> [...] >>>> ;; FIXME: This is ultimately called from >>>> ;; `completion-try-completion' or `completion-all-completions'; >>>> ;; there is only one style currently being used. Need to pass that >>>> ;; style from there to here. >>>> >>>> it only makes sense to call the conversion function corresponding to the >>>> style that was used to generate that string. >>>> >>>> [ Also while a specific call to minibuffer-complete (or >>>> minibuffer-completion-help, or minibuffer-force-complete, ...) only >>>> uses a single style, a given completing-read session can currently use >>>> several completion styles. ] >>>> >>>> So I think we should fix this FIXME before we can move this code to >>>> minibuffer.el. Maybe we can save the completion style that returned >>>> that string in a text-property, or even directly store the conversion >>>> function in there (so we don't need to extend completion-style-alist). >>> >>> ... completion-get-data-string is called after >>> completing-read-default, via uniq-file-completing-read-default-advice. >>> At that point, user-string was computed by the last style tried (ie >>> uniq-file-all-completions, which could set a text property), but I'm not >>> sure if it's copied (losing the text properties) in the middle >>> somewhere. I'll try it. >> >> This is now implemented, in ELPA. > > Except it also needs: > > (setq minibuffer-allow-text-properties t) That's not enough. Despite that setting, when there is a single completion, minibuffer-force-complete calls completion--replace, which explicitly deletes all text properties from the minibuffer text. Then minibuffer-force-complete returns that text. In addition, minibuffer-force-complete unconditionally uses buffer-substring-no-properties to return the text. So that setting lies, and using text properties to pass the style to completion-get-data-string is not reliable. -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-02-15 15:50 ` Stephen Leake 2019-02-15 22:47 ` Stephen Leake @ 2019-04-19 17:49 ` Stephen Leake 2019-05-03 0:48 ` Dmitry Gutov 1 sibling, 1 reply; 103+ messages in thread From: Stephen Leake @ 2019-04-19 17:49 UTC (permalink / raw) To: emacs-devel I've pushed a scratch branch scratch/project-uniquify-files with a different approach to this issue. I did some benchmarking, and realized that almost all of the time taken by uniq-file-completion-table startup is spent scanning the disk for file names, and filling the operating system disk cache. Doing all of that first to get the file list, and then doing completion on the list, is measureably slower than repeating the scan (using the cache) during completion, but the difference is not enough to be noticeable (unless the completion string eliminates most of the directories, which is not a typical case for uniquify-files). Since standard completion works with alists, one way to make the result string different from the display string is to build an alist with the display string as key. Then we just need to fetch the result string from the alist; completing-read does not do that, unfortunately (I guess because the cdr of the alist might not be a string, in general). Also, in browsing around the completion sources, I found the 'quote/unquote' mechanism, which passes yet more actions to the completion table function. So I added 'alist to the completion table metadata, and a step in completing-read-default to query the completion table with the 'alist action if that metadata is present. file-complete-root-relative and uniquify-files both use this mechanism; they wrap an alist in a completion function that handles 'alist. An alternate implementation would be to add a global variable or argument to completing-read to do the alist step, but that would require clients to know that the completion table is an alist. project--completing-read-strict delegates the UI to the completion table, with the default using file-complete-root-relative. I have not updated ELPA to match this, but it should be straight-forward. -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-04-19 17:49 ` Stephen Leake @ 2019-05-03 0:48 ` Dmitry Gutov 2019-05-04 10:39 ` Stephen Leake 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-05-03 0:48 UTC (permalink / raw) To: Stephen Leake, emacs-devel [-- Attachment #1: Type: text/plain, Size: 2834 bytes --] On 19.04.2019 20:49, Stephen Leake wrote: > I've pushed a scratch branch scratch/project-uniquify-files with a > different approach to this issue. > > I did some benchmarking, and realized that almost all of the time taken > by uniq-file-completion-table startup is spent scanning the disk for > file names, and filling the operating system disk cache. Doing all of > that first to get the file list, and then doing completion on the list, > is measureably slower than repeating the scan (using the cache) during > completion, but the difference is not enough to be noticeable (unless > the completion string eliminates most of the directories, which is not a > typical case for uniquify-files). I've seen similar results in my own optimization initiatives. Simple often beats being too clever. > Since standard completion works with alists, one way to make the result > string different from the display string is to build an alist with the > display string as key. I think this is the key insight. And we don't really need complex completion styles or special completion tables (which wouldn't work across project implementations). > Then we just need to fetch the result string from > the alist; completing-read does not do that, unfortunately (I guess > because the cdr of the alist might not be a string, in general). Calling cdr is not hard, though. An extra wrapper can do that. > Also, in browsing around the completion sources, I found the > 'quote/unquote' mechanism, which passes yet more actions to the > completion table function. > > So I added 'alist to the completion table metadata, and a step in > completing-read-default to query the completion table with the 'alist > action if that metadata is present. Like I said before, we don't want project implementations to provide extra features like this by overriding project-file-completion-table. It's not customizable and thus not user-friendly. > file-complete-root-relative and uniquify-files both use this mechanism; > they wrap an alist in a completion function that handles 'alist. All right. Let's try a different mechanism, though. > project--completing-read-strict delegates the UI to the completion > table, with the default using file-complete-root-relative. Let's not do UI inside completion tables. Or as little as possible, at least. Here is my counter-proposal (attached). It both nicely factors out the common-parent-directory logic (which had no place in completing-read-strict anyway) and creates a customization point. You can write a different project-find-file-read-fn that would render the file names differently. For instance, with the alist example, you could build it, call project--completing-read-strict on it, and cdr at the end. Or use a hash-table, etc. Let me know if you need any help with implementing that. [-- Attachment #2: project-find-file-read-fn.diff --] [-- Type: text/x-patch, Size: 4984 bytes --] diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el index 7c8ca15868..70c9f60a0d 100644 --- a/lisp/progmodes/project.el +++ b/lisp/progmodes/project.el @@ -157,19 +157,13 @@ project--find-in-directory vc-directory-exclusion-list) grep-find-ignored-files)) -(cl-defgeneric project-file-completion-table (project dirs) - "Return a completion table for files in directories DIRS in PROJECT. -DIRS is a list of absolute directories; it should be some -subset of the project roots and external roots. - -The default implementation delegates to `project-files'." - (let ((all-files (project-files project dirs))) - (lambda (string pred action) - (cond - ((eq action 'metadata) - '(metadata . ((category . project-file)))) - (t - (complete-with-action action all-files string pred)))))) +(defun project-file--completion-table (all-files) + (lambda (string pred action) + (cond + ((eq action 'metadata) + '(metadata . ((category . project-file)))) + (t + (complete-with-action action all-files string pred))))) (cl-defmethod project-roots ((project (head transient))) (list (cdr project))) @@ -470,19 +464,14 @@ project-or-external-find-file (project-external-roots pr)))) (project-find-file-in (thing-at-point 'filename) dirs pr))) -(defun project-find-file-in (filename dirs project) - "Complete FILENAME in DIRS in PROJECT and visit the result." - (let* ((table (project-file-completion-table project dirs)) - (file (project--completing-read-strict - "Find file" table nil nil - filename))) - (if (string= file "") - (user-error "You didn't specify the file") - (find-file file)))) +(defcustom project-find-file-read-fn #'project-find-file--read-cpd-relative + "Function to call to read a file name from a list. +For the arguments list, see project-find-file--read-cpd-relative." + :type 'function) -(defun project--completing-read-strict (prompt - collection &optional predicate - hist default inherit-input-method) +(defun project-find-file--read-cpd-relative (prompt + collection &optional predicate + hist default) ;; Tried both expanding the default before showing the prompt, and ;; removing it when it has no matches. Neither seems natural ;; enough. Removal is confusing; early expansion makes the prompt @@ -504,21 +493,43 @@ project--completing-read-strict ((eq action 'metadata) (if (functionp collection) (funcall collection nil nil 'metadata))) (t - (complete-with-action action substrings string pred))))) - (new-prompt (if default + (complete-with-action action substrings string pred))))) + (res (project--completing-read-strict prompt + new-collection predicate + hist default))) + (concat common-parent-directory res))) + +(defun project-find-file-in (filename dirs project) + "Complete FILENAME in DIRS in PROJECT and visit the result." + (let* ((all-files (project-files project dirs)) + (table (project-file--completion-table all-files)) + (file (funcall project-find-file-read-fn + "Find file" table nil nil + filename))) + (if (string= file "") + (user-error "You didn't specify the file") + (find-file file)))) + +(defun project--completing-read-strict (prompt + collection &optional predicate + hist default) + ;; Tried both expanding the default before showing the prompt, and + ;; removing it when it has no matches. Neither seems natural + ;; enough. Removal is confusing; early expansion makes the prompt + ;; too long. + (let* ((new-prompt (if default (format "%s (default %s): " prompt default) (format "%s: " prompt))) (res (completing-read new-prompt - new-collection predicate t + collection predicate t nil ;; initial-input - hist default inherit-input-method))) + hist default))) (when (and (equal res default) (not (test-completion res collection predicate))) (setq res (completing-read (format "%s: " prompt) - new-collection predicate t res hist nil - inherit-input-method))) - (concat common-parent-directory res))) + collection predicate t res hist nil))) + res)) (declare-function fileloop-continue "fileloop" ()) ^ permalink raw reply related [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-05-03 0:48 ` Dmitry Gutov @ 2019-05-04 10:39 ` Stephen Leake 2019-05-07 18:02 ` Stephen Leake 0 siblings, 1 reply; 103+ messages in thread From: Stephen Leake @ 2019-05-04 10:39 UTC (permalink / raw) To: emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > Here is my counter-proposal (attached). It both nicely factors out the > common-parent-directory logic (which had no place in > completing-read-strict anyway) and creates a customization point. > > You can write a different project-find-file-read-fn that would render > the file names differently. For instance, with the alist example, you > could build it, call project--completing-read-strict on it, and cdr at > the end. Or use a hash-table, etc. Let me know if you need any help > with implementing that. Looks good; I'm testing this now. -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-05-04 10:39 ` Stephen Leake @ 2019-05-07 18:02 ` Stephen Leake 2019-05-07 22:35 ` Dmitry Gutov 2019-05-14 2:13 ` Dmitry Gutov 0 siblings, 2 replies; 103+ messages in thread From: Stephen Leake @ 2019-05-07 18:02 UTC (permalink / raw) To: emacs-devel Stephen Leake <stephen_leake@stephe-leake.org> writes: > Dmitry Gutov <dgutov@yandex.ru> writes: > >> Here is my counter-proposal (attached). It both nicely factors out the >> common-parent-directory logic (which had no place in >> completing-read-strict anyway) and creates a customization point. >> >> You can write a different project-find-file-read-fn that would render >> the file names differently. For instance, with the alist example, you >> could build it, call project--completing-read-strict on it, and cdr at >> the end. Or use a hash-table, etc. Let me know if you need any help >> with implementing that. > > Looks good; I'm testing this now. This works well for my use cases. It would be slightly more efficient if `project-find-file-read-fn' took an 'all-files' arg instead of 'collection' (which is a completion-table function); I have to do '(all-completions "" collection predicate)' to get the files list to pass to uniq-file-uniquify. The downside of that is other read-file-from-list functions might have to duplicate 'project-file--completion-table' as well. Until we have another example of this, we can't tell which design is better. The difference in startup time is not noticeable, so I can live with it. One nit; 'project-file--completion-table' would be better spelled 'project--file-completion-table'. Similarly for 'project-find-file--read-cpd-relative' -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-05-07 18:02 ` Stephen Leake @ 2019-05-07 22:35 ` Dmitry Gutov 2019-05-08 1:53 ` Stefan Monnier 2019-05-14 2:13 ` Dmitry Gutov 1 sibling, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-05-07 22:35 UTC (permalink / raw) To: Stephen Leake, emacs-devel; +Cc: Stefan Monnier On 07.05.2019 21:02, Stephen Leake wrote: > This works well for my use cases. I'm glad. > It would be slightly more efficient if `project-find-file-read-fn' took > an 'all-files' arg instead of 'collection' (which is a completion-table > function); I have to do '(all-completions "" collection predicate)' to > get the files list to pass to uniq-file-uniquify. I would like to, really. > The downside of that is other read-file-from-list functions might have > to duplicate 'project-file--completion-table' as well. Until we have > another example of this, we can't tell which design is better. The real reason I went with this approach is so that the no-decorator option would also work and use the same completion category. I.e. (setq project-find-file-read-fn project--completing-read-strict) would still work, except without hiding the common parent directory. If project-find-file-read-fn is passed a flat list of files (which I would personally prefer, for clarity), the "bare" option would still have to be a wrapper that creates a completion table just for the sake of passing the appropriate category to completing-read. > The difference in startup time is not noticeable, so I can live with it. all-completions is pretty fast. I would prefer the cleanest possible design here, though. Paging Dr. Monnier. Stefan, what would you choose? > One nit; 'project-file--completion-table' would be better spelled > 'project--file-completion-table'. > > Similarly for 'project-find-file--read-cpd-relative' Yes, thank you. It's a proof-of-concept patch for now, I'll make the spelling edits later. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-05-07 22:35 ` Dmitry Gutov @ 2019-05-08 1:53 ` Stefan Monnier 2019-05-14 2:14 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-05-08 1:53 UTC (permalink / raw) To: emacs-devel > Paging Dr. Monnier. Stefan, what would you choose? Where does it hurt? Have you tried some ibuprofen? Stefan "not really here" ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-05-08 1:53 ` Stefan Monnier @ 2019-05-14 2:14 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-05-14 2:14 UTC (permalink / raw) To: Stefan Monnier, emacs-devel On 08.05.2019 4:53, Stefan Monnier wrote: > Where does it hurt? Have you tried some ibuprofen? Ibuprofen worked quite well. Only some pain in the completion-styles region remains. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-05-07 18:02 ` Stephen Leake 2019-05-07 22:35 ` Dmitry Gutov @ 2019-05-14 2:13 ` Dmitry Gutov 1 sibling, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-05-14 2:13 UTC (permalink / raw) To: Stephen Leake, emacs-devel On 07.05.2019 21:02, Stephen Leake wrote: > It would be slightly more efficient if `project-find-file-read-fn' took > an 'all-files' arg instead of 'collection' (which is a completion-table > function); I have to do '(all-completions "" collection predicate)' to > get the files list to pass to uniq-file-uniquify. OK, I pushed a slightly different patch, the variable is named project-read-file-name-function. > The downside of that is other read-file-from-list functions might have > to duplicate 'project-file--completion-table' as well. Until we have > another example of this, we can't tell which design is better. And the functions do need to take care of constructing the completion table themselves. I think the result looks simpler, though. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-02-12 1:31 ` Stephen Leake 2019-02-15 15:50 ` Stephen Leake @ 2019-02-19 17:45 ` Stefan Monnier 2019-02-20 19:58 ` Stephen Leake 1 sibling, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-02-19 17:45 UTC (permalink / raw) To: emacs-devel > Yes, because a collection should only specify a style if it cannot work > with other styles. I read this to mean that this collection-specified style would only be used by collections which are fundamentally "broken" (because unable to work as collection for any other style). I had the impression that such "broken" collections are very rare. Some collections may be partly broken (work poorly with some styles, e.g. because they can't show all the completions of an empty string), but that it's only/mostly due to the inadequacy of the current collection API and even in those cases, it still works acceptably for several completion styles. We definitely want to support the case where the collection works OK for most/many/some styles, where it also wants to provide its own style, and where we want the user to be able to override that (via completion-category-oiverrides). So even for those collections where only its own style works, it makes sense to use that same (overridable) mechanism rather than provide another. >> I don't understand this. Why can't the completion style compute the >> common-prefix? > The root is _not_ the common prefix of the current matches; I don't mean "of the current matches" but "of the entire collection". (try-completion "" collection) should return just that, so the completion-style does have access to it (unless the collection doesn't implement `try-completion` correctly for that case, but it seems that in the present situation we have control over the collection so we can make sure it works correctly). >> Also, I don't quite understand why we need 2: they both seem to >> implement the same direction of the conversion (i.e. from "user-string" >> to "data-string"). > > No, "table input format" is not the same as "data format". See the > header comments in uniquify-files; "table input" is the format required > for the "string" parameter to the completion table functions; for both > uniquify-file and file-root-rel it is "partial_dir/partial_basename". > "data" is the format returned by all-completions; an absolute file name. IIUC this means you're using a different API than the normal completion-table API (e.g. in the normal API the return value of (try-completion STR COLL) should have STR as prefix and (all-completions STR COLL) should have as prefix the part of STR past the completion-boundary). So that means the interface between your collection and your completion-style is basically private and hence your 3 representation and the functions between them are also internal implementation choices. [ Yes, I realize, I'm kind of stating the obvious, since you said earlier that this collection only works with this style. ] BTW, I have some problems when trying your code: I started Emacs, loaded uniquify-files and then did M-: (locate-uniquified-file '("/usr/bin")) and when I type d? in the minibuffer, I get the list of matching *absolute* files in *Completions*. Is that the intended behavior? More to the point, the "user format" seems identical to the "data format". Am I doing something wrong? >> I see that uniq-file-get-data-string does more (i.e. it tries to find >> the corresponding match if there's only one) but I don't understand >> why you need to do that: this seems to do a kind of completion which >> minibuffer-complete-and-exit should already have done (if >> `require-match` is set) or which shouldn't be done (if `require-match` >> is not set). > > The main computation in uniq-file-get-data-string is to return the > absolute file name corresponding to the table input string. > minibuffer-complete-and-exit can't do that, because it doesn't know how > to convert the user string to the absolute file name; as far as it > knows, "foo-file1.text<Alice/alice-1/>" is not a valid completion for > "c:/.../uniquify-file-resources/Alice/alice-1/foo-file1.text", because > they are not string-equal. What I meant is that when we call uniq-file-get-data-string we already know that the user string is a valid match, so there shouldn't be any need to search in the completion table. E.g. if it's "foo-file1.text<Alice/alice-1/>" we should be able to just prepend the hidden common prefix to the output of uniq-file-to-table-input, no? Later you wrote: > This is now implemented, in ELPA. [...] > Except it also needs: > (setq minibuffer-allow-text-properties t) [...] And then: > That's not enough. Despite that setting, when there is a single > completion, minibuffer-force-complete calls completion--replace, We could fix completion--replace. It currently removes all properties, but that's just because it was easier, I think it only cares about visible properties, so we could be more careful to only remove the `face`, `invisible`, and `display` properties. > In addition, minibuffer-force-complete unconditionally uses > buffer-substring-no-properties to return the text. We could also try and change this. But thinking more about it, relying on text-properties is inconvenient and unnecessary: inconvenient because as we can see they tend to be unreliable (and there's always the chance that the user typed that part of the text anyway), and unnecessary because this kind of completion style (which rearranges the text of completion candidates) can't really be mixed with other styles (the resulting behavior would be rather weird), so we don't need to know which style generated the chunk of text, we just need to know whether our style is the (only) currently active one. I'm starting to wonder if "completion-style" is the right place to put this. E.g. after removing the common prefix and swapping the dir/file to file/dir the remaining completion operations could meaningfully use completion-styles, so maybe it's better thought as an extra layer between the completion style and the collection ... ... or something. Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-02-19 17:45 ` Stefan Monnier @ 2019-02-20 19:58 ` Stephen Leake 2019-02-21 2:00 ` Stefan Monnier 0 siblings, 1 reply; 103+ messages in thread From: Stephen Leake @ 2019-02-20 19:58 UTC (permalink / raw) To: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> Yes, because a collection should only specify a style if it cannot work >> with other styles. > > I read this to mean that this collection-specified style would only be > used by collections which are fundamentally "broken" (because unable to > work as collection for any other style). Yes. Note that uniq-file-completion-table is an optimization; the uniquify-file completion style does work with a list of absolute files, or any completion table function that returns such a list. > I had the impression that such "broken" collections are very rare. Ok. Possibly also undesirable/discouraged? The optimization provided by uniq-file-completion-table (as opposed to a list of all files) is important. Maybe I can improve that function so it works with other styles; more below. path-iterator.el could provide a completion table function that implements the same optimizations, which would work with existing styles. > We definitely want to support the case where the collection works OK for > most/many/some styles, where it also wants to provide its own style, and > where we want the user to be able to override that (via > completion-category-oiverrides). So even for those collections where > only its own style works, it makes sense to use that same (overridable) > mechanism rather than provide another. But then project backends that choose to provide a uniquify-file (or any other "broken") completion table won't "just work"; the user will have to set completion-category-default or -overrides to get completion to work (as illustrated by locate-uniquified-file; more below). >>> I don't understand this. Why can't the completion style compute the >>> common-prefix? >> The root is _not_ the common prefix of the current matches; > > I don't mean "of the current matches" but "of the entire collection". > (try-completion "" collection) should return just that, so the > completion-style does have access to it (unless the collection doesn't > implement `try-completion` correctly for that case, but it seems that > in the present situation we have control over the collection so we can > make sure it works correctly). Yes, that would work, and be horribly inefficient. The initial delay in computing the file list for the emacs source code is already noticeable. Rescanning the entire list on every completion step would be unacceptable. >>> Also, I don't quite understand why we need 2: they both seem to >>> implement the same direction of the conversion (i.e. from "user-string" >>> to "data-string"). >> >> No, "table input format" is not the same as "data format". See the >> header comments in uniquify-files; "table input" is the format required >> for the "string" parameter to the completion table functions; for both >> uniquify-file and file-root-rel it is "partial_dir/partial_basename". >> "data" is the format returned by all-completions; an absolute file name. > > IIUC this means you're using a different API than the normal > completion-table API (e.g. in the normal API the return value of > (try-completion STR COLL) should have STR as prefix and (all-completions > STR COLL) should have as prefix the part of STR past the > completion-boundary). > > So that means the interface between your collection and your > completion-style is basically private and hence your 3 representation > and the functions between them are also internal implementation > choices. I hope this is a extension that could be useful in other situations. Since uniquify-file style works with a plain list collection, your suggestion is not completely true. Ah; note in completion-to-table-input, it converts user to table input format if the table is a function, but to data format if it's a list. Which means if I change uniq-file-completion-table to accept data format strings, it should work with other styles. uniq-file-completion-table must accept both data and table format strings, since converting user to data requires searching the table, passing a partial directory path (ie table input format). It can check to see if the search string is an absolute file name, and adapt accordingly if needed (ie the regex can be anchored or not). > BTW, I have some problems when trying your code: I started Emacs, loaded > uniquify-files and then did > > M-: (locate-uniquified-file '("/usr/bin")) > > and when I type > > d? > > in the minibuffer, I get the list of matching *absolute* files in > *Completions*. This is a symptom of the problem discussed above; you need to set either completion-category-overrides or -defaults to specify uniquify-files to make this work properly. One way to do that is in locate-uniquified-file, as I did for project--completing-read-strict: (defun locate-uniquified-file (&optional path predicate default prompt) (interactive) (let* ((iter (make-path-iterator :user-path-non-recursive (or path load-path))) (table (apply-partially #'uniq-file-completion-table iter)) (table-styles (cdr (assq 'styles (completion-metadata "" table nil)))) (completion-category-overrides (list (list 'project-file (cons 'styles table-styles))))) (completing-read (or prompt "file: ") table predicate t nil nil default) )) In an early version of uniquify-files, it modified completion-category-defaults for project-files when loaded, so locate-uniquified-file worked. But when I added the file-root-rel style, it became clear that needs to be a user choice, not hard-coded. Setting completion-category-overrides in locate-uniquified-file is ok, since the name indicates uniquified style. > What I meant is that when we call uniq-file-get-data-string we already > know that the user string is a valid match, so there shouldn't be any > need to search in the completion table. E.g. if it's > "foo-file1.text<Alice/alice-1/>" we should be able to just prepend the > hidden common prefix to the output of uniq-file-to-table-input, no? Yes, but that common prefix is only stored implicitly in the table. If we had a completion object, we could store it there. Given recent experiments with string properties, it might be possible to store the abs file name as a string property. But I doubt that will work unless we fix minibuffer.el to fully implement minibuffer-allow-text-properties (as you suggest below). > Later you wrote: >> This is now implemented, in ELPA. > [...] >> Except it also needs: >> (setq minibuffer-allow-text-properties t) > [...] > > And then: >> That's not enough. Despite that setting, when there is a single >> completion, minibuffer-force-complete calls completion--replace, > > We could fix completion--replace. It currently removes all > properties, but that's just because it was easier, I think it only cares > about visible properties, so we could be more careful to only remove the > `face`, `invisible`, and `display` properties. Right. >> In addition, minibuffer-force-complete unconditionally uses >> buffer-substring-no-properties to return the text. > > We could also try and change this. Right. There are probably other things that need to be fixed. > But thinking more about it, relying on text-properties is inconvenient > and unnecessary: inconvenient because as we can see they tend to be > unreliable (and there's always the chance that the user typed that part > of the text anyway), and unnecessary because this kind of completion > style (which rearranges the text of completion candidates) can't really > be mixed with other styles (the resulting behavior would be rather > weird), I guess by "mixing with other styles" you mean processing a list of styles; if we had (basic uniquify-files), for <tab> on an empty string 'basic' would present a list of absolute file names, but for <tab> after a string that matches some basename 'uniquify-files' would present uniquified file names. That would be confusing, and I now realize that's why I only specify one completion style. And it explains in more detail what's happening with locate-uniquified-files. > I'm starting to wonder if "completion-style" is the right place > to put this. E.g. after removing the common prefix and swapping the > dir/file to file/dir the remaining completion operations could > meaningfully use completion-styles, so maybe it's better thought as an > extra layer between the completion style and the collection ... > ... or something. The core requirements are: - completing-read return an absolute file name, from a collection of abs file names. - display mimimum directory info in completion options or more generally, user format /= data format Maybe there could be a "uniquify/minimize" step when computing the display list from the result of all-completion. Then all styles would call uniq-file--uniquify (but only for project-file or file category!? not useful for buffers and symbols). Note that uniq-file--uniquify is currently called _almost_ at the end of uniq-file-all-completions; uniq-file--hilit is called after it. hilit is tightly tied to both the style and user string format. If uniq-file--uniquify preserved string properties (I have no idea if it does curerntly), hilit could be called first. That would be required to work with other styles. There would have to be a matching "de-uniquify/maximize" step. It will probably require editing minibuffer.el, but I'll think about it. -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-02-20 19:58 ` Stephen Leake @ 2019-02-21 2:00 ` Stefan Monnier 0 siblings, 0 replies; 103+ messages in thread From: Stefan Monnier @ 2019-02-21 2:00 UTC (permalink / raw) To: emacs-devel >> We definitely want to support the case where the collection works OK for >> most/many/some styles, where it also wants to provide its own style, and >> where we want the user to be able to override that (via >> completion-category-oiverrides). So even for those collections where >> only its own style works, it makes sense to use that same (overridable) >> mechanism rather than provide another. > > But then project backends that choose to provide a uniquify-file (or any > other "broken") completion table won't "just work"; the user will have > to set completion-category-default or -overrides to get completion to > work (as illustrated by locate-uniquified-file; more below). No, indeed we don't want that: we want that the completion-table-provided style is used by default without the user having to take any special steps. But we also want it to be possible for the user to specify some other style to use instead. >>>> I don't understand this. Why can't the completion style compute the >>>> common-prefix? >>> The root is _not_ the common prefix of the current matches; >> >> I don't mean "of the current matches" but "of the entire collection". >> (try-completion "" collection) should return just that, so the >> completion-style does have access to it (unless the collection doesn't >> implement `try-completion` correctly for that case, but it seems that >> in the present situation we have control over the collection so we can >> make sure it works correctly). > Yes, that would work, and be horribly inefficient. Indeed, it would only work acceptably if either the backend special cases the "" case or the style is able to do enough caching. >> What I meant is that when we call uniq-file-get-data-string we already >> know that the user string is a valid match, so there shouldn't be any >> need to search in the completion table. E.g. if it's >> "foo-file1.text<Alice/alice-1/>" we should be able to just prepend the >> hidden common prefix to the output of uniq-file-to-table-input, no? > Yes, but that common prefix is only stored implicitly in the table. > If we had a completion object, we could store it there. Ah, OK, so it's an artifact of limitations of the current API, makes sense. > I guess by "mixing with other styles" you mean processing a list of > styles; if we had (basic uniquify-files), for <tab> on an empty string > 'basic' would present a list of absolute file names, but for <tab> after > a string that matches some basename 'uniquify-files' would present > uniquified file names. That would be confusing, and I now realize that's > why I only specify one completion style. Exactly: they're mutually incompatible (not necessary in the sense that something will break, but in the sense that the result makes no sense for the user). > It will probably require editing minibuffer.el, but I'll think about it. Right, I'm thinking from the point of view that minibuffer.el can be changed at will. If/when we figure out a clean solution there, we can maybe think about how to try and make it work with current minibuffer.el. > Note that uniq-file-completion-table is an optimization; the > uniquify-file completion style does work with a list of absolute files, > or any completion table function that returns such a list. Indeed, thinking about the kind of manipulation done for your uniquification, it seems like it pretty much requires to have the complete list from the start (and to basically start by rewriting this list from the data format to the user format on entry, then do normal completion on this and then convert the final result back to data format). Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-18 2:19 ` Dmitry Gutov 2019-01-18 3:05 ` Stefan Monnier 2019-01-21 19:32 ` Stephen Leake @ 2019-01-21 19:36 ` Stephen Leake 2019-01-22 0:20 ` Dmitry Gutov 2 siblings, 1 reply; 103+ messages in thread From: Stephen Leake @ 2019-01-21 19:36 UTC (permalink / raw) To: emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > Would you agree that a "flat completion table" is basically always a > list of files? The completion table in uniquify files completes on base names first, then on uniquifying directories. Given an empty completion string, it will return a list of all files (as absolute file names), but completing on that is not equivalent to using the table. I don't know whether that completion table is "flat". We could define "flat" to mean 'equivalent to a list of files"; that seems like a useful definition. -- -- Stephe ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: project--completing-read-strict breaks ada-mode project completion table 2019-01-21 19:36 ` Stephen Leake @ 2019-01-22 0:20 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-22 0:20 UTC (permalink / raw) To: Stephen Leake, emacs-devel On 21.01.2019 22:36, Stephen Leake wrote: > The completion table in uniquify files completes on base names first, > then on uniquifying directories. This table won't be returned by a generic, though, right? It will be constructed inside project-find-file, depending on a particular value of a new user option. And the function that builds it will need take a list of files as input, right? > Given an empty completion string, it will return a list of all files (as > absolute file names), but completing on that is not equivalent to using > the table. > > I don't know whether that completion table is "flat". We could define > "flat" to mean 'equivalent to a list of files"; that seems like a useful > definition. OK, let's call it that. Not sure if we're going to use this definition a lot in the future, though. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: Making project-files the "canonical" generic 2019-01-15 1:14 ` Stephen Leake 2019-01-16 16:38 ` Stefan Monnier 2019-01-16 19:02 ` project--completing-read-strict breaks ada-mode project completion table Stephen Leake @ 2019-01-17 3:04 ` Dmitry Gutov 2 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-17 3:04 UTC (permalink / raw) To: Stephen Leake, emacs-devel On 15.01.2019 04:14, Stephen Leake wrote: >> What is your use for completion table? I mean, the ability to provide >> your implementation. The default implementation should already work if >> project-roots and project-ignores return the correct values. > > In my case, they don't; as I have said elsewhere, the Ada compiler tools > require flat paths, so the Ada project code doesn't use project-roots, > project-ignores. I have code that converts between the two, but calling > that for every use of project-files would be inefficient. Why is that code so inefficient? Can't you get by with a limited list of ignores? I don't see how it can work otherwise. project-files *cannot* describe different information than what project-roots combined with project-ignores do. Any API consumer should be allowed to use any of those function, not just project-files or project-file-completion-table. Does the current implementation of project-find-regexp even work for you (it doesn't use the completion table)? And if so, how? > Providing a more flexible definition of project-path would fix that > problem. We have discussed this before. How would this work? Say, we have project-find-regexp which uses project-roots and project-ignores to build its command. How would it work with a "more flexible definition"? >> The only big reason to do that that I know of, is to use a cached list >> of files (or somehow fetch it quickly another way, without 'find'). >> But project-files is just as suitable for that. > > I could override project-files to return the correct list. Yes. I think I'd rather you did that and stopped there (aside from the project-roots/project-ignores problem). > The completion table style I use does uniquification by appending enough > parent directories to make the file names unique (same as > uniquify-buffer-name-style post-forward-angle-brackets). I might be able > to make that work with the list returned by project-files, but currently > some of the work is done in the completion table code itself, as it is > traversing the directory path. When I wrote it, I tried but did not > succeed in keeping all the completion-style-dependent code out of the > table. Maybe things have changed. I have commented on this in another email, and I understand how you reached that solution. And I don't like it, sorry. Doing all this using a completion style seems impossible, but you could implement the uniquification inside the (new) command, not inside the completion table. Then we'll have two commands for the same purpose, project-find-file and project-????-find-file. After, we could also try to combine them and make the behavior depend on the user preference. The completion table seems like the wrong way to do it, though. > The completion table is also lazy, in the sense that typing characters > in the completion buffer interrupts computation of the table, and it is > later resumed properly. I'm not sure "lazy" is the right term for this; > perhaps "interruptible"? I'm not clear if that mechanism would work thru > project-files. Err, I'm not sure how this works, but I think a project-files implementation could be "interruptible" as well. >> project-files is now used for project-find-file, and will be used for >> project-find-regexp as well. > > ? not in master as of 492ab1136860ef41c33a5e35a85fb721693f892b, fetched > today. Huh, you are right, sorry. It will be used for the default completion table, though. And I have plans for it to be used in project-find-regexp as well. >>> Do we have any examples of that? find-file works >>> that way, but it's not a completion table. >> >> ecomplete-completion-table is the one example that I found quickly, >> but there are probably more. > > ok, so there are real examples that require project-files and > project-file-completion-table to be independent of each other. I don't follow. The fact that some completion tables are kind of tricky doesn't mean we need to allow them here. We should find some specific use to them for our particular purpose. In any case, the comments that mention the "flat" bit are about how difficult those table make it for the commands that try to use them. Which is not an encouragement for having them supported. > And the user interface experience for completion tables is not fully > determined by completion-styles-alist, so my uniquifying completion > table above is not really an outlier. Having UI depend on a backend is not really a great idea, IMO. > But defining a non-flat table on top of project-files is not possible, > or at least very inefficient (you'd have to throw away all the nested > directory content). Could you elaborate on that? What have you tried, and how did it it fail to work? > Low level facilities like project.el should enable _all_ possible use > cases, not rule out some in the name of simplicity. All _feasible_ use cases. The said facility should itself remain usable and discourage buggy code. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-27 1:49 ` Dmitry Gutov 2018-12-27 14:39 ` Stefan Monnier @ 2018-12-27 20:33 ` Juri Linkov 2018-12-27 23:31 ` Dmitry Gutov 2018-12-28 18:07 ` Stefan Monnier 1 sibling, 2 replies; 103+ messages in thread From: Juri Linkov @ 2018-12-27 20:33 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel > And I'm thinking about this because there can be faster ways to list all > files in the project than 'find' (e.g. 'git ls-files'). But > xref-collect-matches should know nothing about 'git ls-files'. 'git ls-files' seems very fast, and moreover it outputs only relative paths, not absolute. On TAB completion with too long absolute paths the list of completions is quite unreadable. Also is it possible to complete only on file names, not paths? > OK, so unless somebody objects I'd like to move them to lisp/multifile.el > and rename to multifile-project-find-regexp and > multifile-project-query-replace-regexp. I think they should mirror everything that makes sense to use in the multifile project: project-occur, project-grep, ... Since there is query-replace-regexp, multifile-project-query-replace-regexp makes sense. But I don't know to what existing command corresponds 'project-search'? I tried it on the Emacs source tree, but it failed with: Debugger entered--Lisp error: (compression-error "Opening input file" "error uncompressing empty.zz.gz" "emacs/test/manual/etags/a-src/empty.zz.gz") ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-27 20:33 ` [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el Juri Linkov @ 2018-12-27 23:31 ` Dmitry Gutov 2018-12-27 23:45 ` Juri Linkov 2018-12-28 18:07 ` Stefan Monnier 1 sibling, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2018-12-27 23:31 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel On 27.12.2018 22:33, Juri Linkov wrote: > 'git ls-files' seems very fast, and moreover it outputs only relative > paths, not absolute. On TAB completion with too long absolute paths > the list of completions is quite unreadable. That's a question of presentation, not data. And Stefan gave some pertinent suggestions in this thread. Actually, if we didn't manage to have 'find' and 'git ls-files' output in the same format as each other, that would be a problem. > Also is it possible to complete only on file names, not paths? You can input a file name and press RET. If the name is unique, it will be completed fully. Isn't this the best possible scenario? >> OK, so unless somebody objects I'd like to move them to lisp/multifile.el >> and rename to multifile-project-find-regexp and >> multifile-project-query-replace-regexp. > > I think they should mirror everything that makes sense to use in the > multifile project: project-occur, project-grep, ... "multifile project" Stefan, I think Juri is (maybe unknowingly) hinting that the package's name is a bit unfortunate. Every project is multifile (with very rare exceptions). It's not about that. Or maybe I don't understand the suggestion. I'm find with adding project-occur, whatever it might do. As for project-grep, we have project-find-regexp already. I think it's a better name. > But I don't know to what existing command corresponds 'project-search'? > I tried it on the Emacs source tree, but it failed with: It's analogous to tags-search. Which is also an unfortunate name, IMO. > Debugger entered--Lisp error: (compression-error "Opening input file" "error uncompressing empty.zz.gz" "emacs/test/manual/etags/a-src/empty.zz.gz") Does M-x project-find-regexp work in the same situation? ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-27 23:31 ` Dmitry Gutov @ 2018-12-27 23:45 ` Juri Linkov 2018-12-28 6:04 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Juri Linkov @ 2018-12-27 23:45 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel >> Also is it possible to complete only on file names, not paths? > > You can input a file name and press RET. If the name is unique, it will be > completed fully. Isn't this the best possible scenario? Rather it completes on path components too, not only on file names. Makes more difficult to match part of the file name when the same substring occurs in the path. >>> OK, so unless somebody objects I'd like to move them to lisp/multifile.el >>> and rename to multifile-project-find-regexp and >>> multifile-project-query-replace-regexp. >> >> I think they should mirror everything that makes sense to use in the >> multifile project: project-occur, project-grep, ... > > "multifile project" > > Stefan, I think Juri is (maybe unknowingly) hinting that the package's name > is a bit unfortunate. > > Every project is multifile (with very rare exceptions). It's not about that. Either prefix multifile- or project- is fine, but not both at the same time. Or better just shorten to multi-. We already have multi-isearch (not supporting project yet). > Does M-x project-find-regexp work in the same situation? I see that M-x project-find-regexp is like occur, so a better name would be M-x project-occur. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-27 23:45 ` Juri Linkov @ 2018-12-28 6:04 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2018-12-28 6:04 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel On 28.12.2018 1:45, Juri Linkov wrote: > Rather it completes on path components too, not only on file names. > Makes more difficult to match part of the file name when the same substring > occurs in the path. But base file names can also have duplicates, right? > Either prefix multifile- or project- is fine, but not both at the same time. > Or better just shorten to multi-. We already have multi-isearch (not > supporting project yet). OK, let me put it another way: "multifile" is just a package that implements a particular UI. It is in no way synonymous with "project". Maybe a better name for it would be something like bufferloop (suggestions welcome). >> Does M-x project-find-regexp work in the same situation? > > I see that M-x project-find-regexp is like occur, so a better name would be > M-x project-occur. I can see where you're coming from, but we've already discussed that it has different behavior from Occur. And it uses different data sources. I agree that some unification might be in order, but naming things the same won't cut it. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-27 20:33 ` [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el Juri Linkov 2018-12-27 23:31 ` Dmitry Gutov @ 2018-12-28 18:07 ` Stefan Monnier 2018-12-29 0:31 ` Dmitry Gutov 2018-12-29 22:02 ` Juri Linkov 1 sibling, 2 replies; 103+ messages in thread From: Stefan Monnier @ 2018-12-28 18:07 UTC (permalink / raw) To: emacs-devel Juri wrote: > 'git ls-files' seems very fast, and moreover it outputs only relative > paths, not absolute. [ Nitpick: the GNU convention says these are "file names" rather than "paths". ] > On TAB completion with too long absolute paths > the list of completions is quite unreadable. That's why I was suggesting to start by stripping the common prefix. > Also is it possible to complete only on file names, not paths? With the `substring` completion style, yes. > I think they should mirror everything that makes sense to use in the > multifile project: project-occur, project-grep, ... occur operates on buffers, not files, so I don't think mirroring it into multifile- or project- makes much sense. The corresponding command for files is `grep`, so `project-grep` might make sense. Dmitry wrote: > Stefan, I think Juri is (maybe unknowingly) hinting that the package's name > is a bit unfortunate. I'm not particularly proud of that name. All I wanted was the multifile-query-replace feature, and "multifile" just was the least bad name I could come up with. That's also why I chose the name `project-query-replace` over `multifile-query-replace` or anything like that: the fact that it uses multifile.el is an internal detail, IMO. Juri wrote: > Either prefix multifile- or project- is fine, but not both at the same time. > Or better just shorten to multi-. We already have multi-isearch (not > supporting project yet). I chose "project-" so it actually says to which files it is supposed to apply, compared to "multi-" or "multifile-" which just says that it applies to several files but without clarifying which are those. Dmitry wrote: > OK, let me put it another way: "multifile" is just a package that implements > a particular UI. It is in no way synonymous with "project". Maybe a better > name for it would be something like bufferloop (suggestions welcome). multifile loops over several *files* rather than buffers, so I'm fine with "fileloop" or "iteratefiles", but "bufferloop" doesn't seem right. Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-28 18:07 ` Stefan Monnier @ 2018-12-29 0:31 ` Dmitry Gutov 2018-12-29 22:02 ` Juri Linkov 1 sibling, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2018-12-29 0:31 UTC (permalink / raw) To: Stefan Monnier, emacs-devel On 28.12.2018 20:07, Stefan Monnier wrote: > That's also why I chose the name `project-query-replace` over > `multifile-query-replace` or anything like that: the fact that it uses > multifile.el is an internal detail, IMO. It is an intentional choice as well, however. At least, like dired-do-find-regexp-and-replace demonstrates, the xref UI can also be used for search and replace. > multifile loops over several *files* rather than buffers, so I'm fine > with "fileloop" or "iteratefiles", but "bufferloop" doesn't seem right. "fileloop" sounds good. Other similar options I had are iterfile and filiter. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-28 18:07 ` Stefan Monnier 2018-12-29 0:31 ` Dmitry Gutov @ 2018-12-29 22:02 ` Juri Linkov 2018-12-30 23:13 ` Dmitry Gutov 1 sibling, 1 reply; 103+ messages in thread From: Juri Linkov @ 2018-12-29 22:02 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel >> 'git ls-files' seems very fast, and moreover it outputs only relative >> paths, not absolute. > > [ Nitpick: the GNU convention says these are "file names" rather than > "paths". ] Sorry, I remembered the convention regarding file names and paths, but I forgot how to name the base file name part as opposed to its directory part. Now I consulted in the Glossary it's called File-Name Component. >> On TAB completion with too long absolute paths >> the list of completions is quite unreadable. > > That's why I was suggesting to start by stripping the common prefix. > >> Also is it possible to complete only on file names, not paths? > > With the `substring` completion style, yes. I meant something more like switch-to-buffer, but that completes on project file names (project-switch-to-buffer makes sense as well to complete on already visited project buffers). So a new command project-switch-to-file could complete on non-directory file name components from the project. And on duplicate file names it could add a unique suffix '<sub/dir>' like is used to make buffer names unique. Then completions will show project file names in alphabetical order. >> I think they should mirror everything that makes sense to use in the >> multifile project: project-occur, project-grep, ... > > occur operates on buffers, not files, so I don't think mirroring it into > multifile- or project- makes much sense. The corresponding command for > files is `grep`, so `project-grep` might make sense. project-occur could operate only on visited project files (but I doubt if this is useful). project-rgrep is much more needed to operate like rgrep, but without asking for file names and root directory. >> Either prefix multifile- or project- is fine, but not both at the same time. >> Or better just shorten to multi-. We already have multi-isearch (not >> supporting project yet). > > I chose "project-" so it actually says to which files it is supposed to > apply, compared to "multi-" or "multifile-" which just says that it > applies to several files but without clarifying which are those. > > Dmitry wrote: >> OK, let me put it another way: "multifile" is just a package that implements >> a particular UI. It is in no way synonymous with "project". Maybe a better >> name for it would be something like bufferloop (suggestions welcome). > > multifile loops over several *files* rather than buffers, so I'm fine with > "fileloop" or "iteratefiles", but "bufferloop" doesn't seem right. "hulahoop" :) Actually I think the existing names are already good enough: "multifile" for the package that supports multifile operations, and "project" for UI that operates on project files. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-29 22:02 ` Juri Linkov @ 2018-12-30 23:13 ` Dmitry Gutov 2019-01-02 22:11 ` Juri Linkov 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2018-12-30 23:13 UTC (permalink / raw) To: Juri Linkov, Stefan Monnier; +Cc: emacs-devel On 30.12.2018 1:02, Juri Linkov wrote: > So a new command project-switch-to-file could complete on non-directory > file name components from the project. And on duplicate file names > it could add a unique suffix '<sub/dir>' like is used to make buffer names > unique. Then completions will show project file names in alphabetical order. You can already type 'sub/dir' and press TAB to see it completed. Especially if it's unique. > project-occur could operate only on visited project files > (but I doubt if this is useful). project-rgrep is much more needed > to operate like rgrep, but without asking for file names and root directory. We already have project-find-regexp. Why bother? > "hulahoop" :) I don't mind. > Actually I think the existing names are already good enough: "multifile" > for the package that supports multifile operations, It's not the only package that "supports multifile operations". xref does it as well (and arguably better), and also occur and grep, as some examples. > and "project" > for UI that operates on project files. project is not a UI, it uses one of the other packages for UIs. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2018-12-30 23:13 ` Dmitry Gutov @ 2019-01-02 22:11 ` Juri Linkov 2019-01-02 23:23 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Juri Linkov @ 2019-01-02 22:11 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel >> So a new command project-switch-to-file could complete on non-directory >> file name components from the project. And on duplicate file names >> it could add a unique suffix '<sub/dir>' like is used to make buffer names >> unique. Then completions will show project file names in alphabetical order. > > You can already type 'sub/dir' and press TAB to see it > completed. Especially if it's unique. That's the problem. Most of the time there is no need to complete on directory names, only on file names, preferably listed alphabetically. >> project-occur could operate only on visited project files >> (but I doubt if this is useful). project-rgrep is much more needed >> to operate like rgrep, but without asking for file names and root directory. > > We already have project-find-regexp. Why bother? Why not? Different users have different preferences. >> Actually I think the existing names are already good enough: "multifile" >> for the package that supports multifile operations, > > It's not the only package that "supports multifile operations". xref does > it as well (and arguably better), and also occur and grep, as > some examples. I don't mind moving the existing multi-occur and other multi- commands to multifile.el. >> and "project" for UI that operates on project files. > > project is not a UI, it uses one of the other packages for UIs. I meant that project is at a higher level than multifile. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-02 22:11 ` Juri Linkov @ 2019-01-02 23:23 ` Dmitry Gutov 2019-01-03 0:44 ` Juri Linkov 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-02 23:23 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel On 03.01.2019 1:11, Juri Linkov wrote: > That's the problem. Most of the time there is no need to complete > on directory names, only on file names, preferably listed alphabetically. So type the file name (or a part of it)? I'm not seeing the problem, sorry. But your particular preference here might be solved with implementing a new completion style. Please go ahead, if you like. >>> project-occur could operate only on visited project files >>> (but I doubt if this is useful). project-rgrep is much more needed >>> to operate like rgrep, but without asking for file names and root directory. >> >> We already have project-find-regexp. Why bother? > > Why not? Different users have different preferences. *shrug*, if you insist. project-grep can work. Even though project-find-regexp is also "project grep", in a sense. >>> Actually I think the existing names are already good enough: "multifile" >>> for the package that supports multifile operations, >> >> It's not the only package that "supports multifile operations". xref does >> it as well (and arguably better), and also occur and grep, as >> some examples. > > I don't mind moving the existing multi-occur and other multi- commands > to multifile.el. WHYYY multifile.el just implements a particular UI. multi-occur is different from occur in a totally other way. Going by your suggestion, we'd cram half of Emacs into that one file. >>> and "project" for UI that operates on project files. >> >> project is not a UI, it uses one of the other packages for UIs. > > I meant that project is at a higher level than multifile. Is "fetching data" a higher level than "displaying data"? I'm not sure. Anyway, project.el is about projects, their files, and so on. You were probably referring to interactive commands, but those can live anywhere. Whereas multifile.el is about iterating through a list of files in a loop with a particular bare-bones UI and a set of commands that should be familiar to older Emacs users. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-02 23:23 ` Dmitry Gutov @ 2019-01-03 0:44 ` Juri Linkov 2019-01-03 11:52 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Juri Linkov @ 2019-01-03 0:44 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel >> That's the problem. Most of the time there is no need to complete >> on directory names, only on file names, preferably listed alphabetically. > > So type the file name (or a part of it)? I'm not seeing the problem, sorry. Typing a part of the file name matches also directory names and all files that these directories contain. > multifile.el just implements a particular UI. > > multi-occur is different from occur in a totally other way. > > Going by your suggestion, we'd cram half of Emacs into that one file. I don't mind adding more multifile stuff to multifile.el, whereas naming it something that contains the word “loop” will limit its scope to only iteration commands. BTW, wanted to ask a related question: in multifile.el I see multifile-continue, but can't find its keybinding. Is it bound to some key like it was earlier IIRC to ‘M-,’? ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-03 0:44 ` Juri Linkov @ 2019-01-03 11:52 ` Dmitry Gutov 2019-01-03 15:35 ` Stefan Monnier 2019-01-03 20:57 ` Juri Linkov 0 siblings, 2 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-01-03 11:52 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel On 03.01.2019 3:44, Juri Linkov wrote: >> So type the file name (or a part of it)? I'm not seeing the problem, sorry. > > Typing a part of the file name matches also directory names > and all files that these directories contain. I see. Well, please refer to the "new completion style" part of this discussion. > I don't mind adding more multifile stuff to multifile.el, > whereas naming it something that contains the word “loop” > will limit its scope to only iteration commands. There is no problem in that: the package indeed has a limited scope, because it contains a particular UI. That's all. I'm pretty exasperated at this point of discussion, so I don't see a good next step here except to go and rename it myself. Or, well, moving the new commands there (and outside of project.el) and forgetting all about it. I might start with that. > BTW, wanted to ask a related question: in multifile.el I see > multifile-continue, but can't find its keybinding. Is it > bound to some key like it was earlier IIRC to ‘M-,’? I think the idea is that the user sets up a binding themselves. E.g. people who don't want to use xref will change 'M-,' back to multifile-continue. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-03 11:52 ` Dmitry Gutov @ 2019-01-03 15:35 ` Stefan Monnier 2019-01-03 23:06 ` Dmitry Gutov 2019-01-03 20:57 ` Juri Linkov 1 sibling, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-01-03 15:35 UTC (permalink / raw) To: emacs-devel >> I don't mind adding more multifile stuff to multifile.el, >> whereas naming it something that contains the word “loop” >> will limit its scope to only iteration commands. > There is no problem in that: the package indeed has a limited scope, because > it contains a particular UI. That's all. Indeed. My conclusion from this discussion is that multifile.el should not accrue commands like those of project.el: it's a library that can be used by things like tags.el or project.el, with a limited scope. Renaming it to a less attractive name is probably a good idea. Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-03 15:35 ` Stefan Monnier @ 2019-01-03 23:06 ` Dmitry Gutov 2019-02-07 12:23 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-03 23:06 UTC (permalink / raw) To: Stefan Monnier, emacs-devel On 03.01.2019 18:35, Stefan Monnier wrote: > Indeed. My conclusion from this discussion is that multifile.el should > not accrue commands like those of project.el: it's a library that can be > used by things like tags.el or project.el, with a limited scope. I agree with that. But still, I'm very tempted to rename project-search to fileloop-find-regexp-in-project and project-query-replace to fileloop-query-replace-regexp-in-project. > Renaming it to a less attractive name is probably a good idea. Or a more specific one, at least. I'd argue that fileloop is not particularly less "attractive" than multifile. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-03 23:06 ` Dmitry Gutov @ 2019-02-07 12:23 ` Dmitry Gutov 2019-02-07 13:05 ` Stefan Monnier 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-02-07 12:23 UTC (permalink / raw) To: Stefan Monnier, emacs-devel [-- Attachment #1: Type: text/plain, Size: 667 bytes --] On 04.01.2019 02:06, Dmitry Gutov wrote: > On 03.01.2019 18:35, Stefan Monnier wrote: > >> Indeed. My conclusion from this discussion is that multifile.el should >> not accrue commands like those of project.el: it's a library that can be >> used by things like tags.el or project.el, with a limited scope. > > I agree with that. But still, I'm very tempted to rename project-search > to fileloop-find-regexp-in-project and project-query-replace to > fileloop-query-replace-regexp-in-project. So... which of these changes do you prefer? Inserting -fileloop- to the names of he commands, or moving them to the fileloop package altogether? Two diffs attached. [-- Attachment #2: project-fileloop.diff --] [-- Type: text/x-patch, Size: 848 bytes --] diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el index fbf761c60c..dea2cd30d6 100644 --- a/lisp/progmodes/project.el +++ b/lisp/progmodes/project.el @@ -522,7 +522,7 @@ project--completing-read-strict (declare-function fileloop-continue "fileloop" ()) ;;;###autoload -(defun project-search (regexp) +(defun project-fileloop-search (regexp) "Search for REGEXP in all the files of the project. Stops when a match is found. To continue searching for next match, use command \\[fileloop-continue]." @@ -532,7 +532,7 @@ project-search (fileloop-continue)) ;;;###autoload -(defun project-query-replace (from to) +(defun project-fileloop-query-replace (from to) "Search for REGEXP in all the files of the project. Stops when a match is found. To continue searching for next match, use command \\[fileloop-continue]." [-- Attachment #3: fileloop-project.diff --] [-- Type: text/x-patch, Size: 2436 bytes --] diff --git a/lisp/fileloop.el b/lisp/fileloop.el index 2e77811a57..b07b81bc1e 100644 --- a/lisp/fileloop.el +++ b/lisp/fileloop.el @@ -213,5 +213,28 @@ fileloop-initialize-replace (lambda () (perform-replace from to t t delimited nil multi-query-replace-map)))) +;;;###autoload +(defun fileloop-search-in-project (regexp) + "Search for REGEXP in all the files of the project. +Stops when a match is found. +To continue searching for next match, use command \\[fileloop-continue]." + (interactive "sSearch (regexp): ") + (fileloop-initialize-search + regexp (project-files (project-current t)) 'default) + (fileloop-continue)) + +;;;###autoload +(defun fileloop-query-replace-in-project (from to) + "Search for REGEXP in all the files of the project. +Stops when a match is found. +To continue searching for next match, use command \\[fileloop-continue]." + (interactive + (pcase-let ((`(,from ,to) + (query-replace-read-args "Query replace (regexp)" t t))) + (list from to))) + (fileloop-initialize-replace + from to (project-files (project-current t)) 'default) + (fileloop-continue)) + (provide 'fileloop) ;;; fileloop.el ends here diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el index fbf761c60c..e526249d48 100644 --- a/lisp/progmodes/project.el +++ b/lisp/progmodes/project.el @@ -519,30 +519,5 @@ project--completing-read-strict inherit-input-method))) (concat common-parent-directory res))) -(declare-function fileloop-continue "fileloop" ()) - -;;;###autoload -(defun project-search (regexp) - "Search for REGEXP in all the files of the project. -Stops when a match is found. -To continue searching for next match, use command \\[fileloop-continue]." - (interactive "sSearch (regexp): ") - (fileloop-initialize-search - regexp (project-files (project-current t)) 'default) - (fileloop-continue)) - -;;;###autoload -(defun project-query-replace (from to) - "Search for REGEXP in all the files of the project. -Stops when a match is found. -To continue searching for next match, use command \\[fileloop-continue]." - (interactive - (pcase-let ((`(,from ,to) - (query-replace-read-args "Query replace (regexp)" t t))) - (list from to))) - (fileloop-initialize-replace - from to (project-files (project-current t)) 'default) - (fileloop-continue)) - (provide 'project) ;;; project.el ends here ^ permalink raw reply related [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-02-07 12:23 ` Dmitry Gutov @ 2019-02-07 13:05 ` Stefan Monnier 2019-02-14 1:11 ` Dmitry Gutov 0 siblings, 1 reply; 103+ messages in thread From: Stefan Monnier @ 2019-02-07 13:05 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel >>> Indeed. My conclusion from this discussion is that multifile.el should >>> not accrue commands like those of project.el: it's a library that can be >>> used by things like tags.el or project.el, with a limited scope. >> >> I agree with that. But still, I'm very tempted to rename project-search to >> fileloop-find-regexp-in-project and project-query-replace to >> fileloop-query-replace-regexp-in-project. > > So... which of these changes do you prefer? > > Inserting -fileloop- to the names of he commands, or moving them to the > fileloop package altogether? As quoted above, I think multifile.el (or whatever it is renamed to) should stay as an Elisp library, and commands built on it belong elsewhere. So I'd prefer if we just add "-<foo>" to the name of the commands. Stefan ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-02-07 13:05 ` Stefan Monnier @ 2019-02-14 1:11 ` Dmitry Gutov 0 siblings, 0 replies; 103+ messages in thread From: Dmitry Gutov @ 2019-02-14 1:11 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 07.02.2019 16:05, Stefan Monnier wrote: > So I'd prefer if we just add "-<foo>" to the name of the commands. Looking also at Dired, it has similarly named fileloop versions of commands: dired-do-search and dired-do-query-replace-regexp that use fileloop, and dired-do-find-regexp and dired-do-find-regexp-and-replace that use find+grep and visualize using xref. So I guess either I have to rename all four fileloop-based commands, or just let them be. The latter seems very attractive now. I did rename project-query-replace to project-query-replace-regexp, though. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-03 11:52 ` Dmitry Gutov 2019-01-03 15:35 ` Stefan Monnier @ 2019-01-03 20:57 ` Juri Linkov 2019-01-03 23:21 ` Dmitry Gutov 1 sibling, 1 reply; 103+ messages in thread From: Juri Linkov @ 2019-01-03 20:57 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel > There is no problem in that: the package indeed has a limited scope, > because it contains a particular UI. That's all. While looking for a good file name we have also to think about good prefixes for function names, e.g. tags-loop-continue - old command bound to M-, multifile-continue - current name file-loop-continue - this new name makes the most sense >> BTW, wanted to ask a related question: in multifile.el I see >> multifile-continue, but can't find its keybinding. Is it >> bound to some key like it was earlier IIRC to ‘M-,’? > > I think the idea is that the user sets up a binding themselves. E.g. people > who don't want to use xref will change 'M-,' back to multifile-continue. But what to do for users who wants to use both? Maybe we should support next-error, so ‘M-g M-n’ will call multifile-continue. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-03 20:57 ` Juri Linkov @ 2019-01-03 23:21 ` Dmitry Gutov 2019-01-05 22:12 ` Juri Linkov 0 siblings, 1 reply; 103+ messages in thread From: Dmitry Gutov @ 2019-01-03 23:21 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel On 03.01.2019 23:57, Juri Linkov wrote: > file-loop-continue - this new name makes the most sense If the package is fileloop, it'll be called fileloop-continue, I think. >> I think the idea is that the user sets up a binding themselves. E.g. people >> who don't want to use xref will change 'M-,' back to multifile-continue. > > But what to do for users who wants to use both? They can also use different bindings, of course. > Maybe we should support next-error, so ‘M-g M-n’ will call multifile-continue. Maybe. I still haven't looked at your latest change in that area (very sorry about that). So I'm not sure what are odds of a conflict between different next-error-supporting facilities now, which would be a possible downside. ^ permalink raw reply [flat|nested] 103+ messages in thread
* Re: [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el 2019-01-03 23:21 ` Dmitry Gutov @ 2019-01-05 22:12 ` Juri Linkov 0 siblings, 0 replies; 103+ messages in thread From: Juri Linkov @ 2019-01-05 22:12 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel >> Maybe we should support next-error, so ‘M-g M-n’ will call multifile-continue. > > Maybe. > > I still haven't looked at your latest change in that area (very sorry about > that). So I'm not sure what are odds of a conflict between different > next-error-supporting facilities now, which would be a possible downside. Currently the most reliable way to avoid conflicts is to rely on the buffer with a list of results. Does project-search produce such a buffer, at least internally? ^ permalink raw reply [flat|nested] 103+ messages in thread
end of thread, other threads:[~2019-05-14 2:14 UTC | newest] Thread overview: 103+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <20180922154639.23195.66360@vcs0.savannah.gnu.org> [not found] ` <20180922154640.9D58220310@vcs0.savannah.gnu.org> 2018-12-26 3:34 ` [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el Dmitry Gutov 2018-12-26 20:13 ` Stefan Monnier 2018-12-27 1:49 ` Dmitry Gutov 2018-12-27 14:39 ` Stefan Monnier 2018-12-28 3:45 ` Dmitry Gutov 2018-12-31 11:42 ` Dmitry Gutov 2018-12-31 15:12 ` Eli Zaretskii 2019-01-02 23:47 ` Dmitry Gutov 2019-01-02 1:49 ` Stefan Monnier 2019-01-03 0:41 ` Dmitry Gutov 2019-01-02 21:53 ` Juri Linkov 2019-01-02 23:02 ` Dmitry Gutov 2019-01-03 0:37 ` Juri Linkov 2019-01-03 11:45 ` Dmitry Gutov 2019-01-03 20:53 ` Juri Linkov 2019-01-06 1:22 ` Dmitry Gutov 2019-01-07 23:22 ` Dmitry Gutov 2019-01-07 23:27 ` Dmitry Gutov 2019-01-08 14:21 ` Michael Albinus 2019-01-08 23:06 ` Dmitry Gutov 2019-01-09 8:10 ` Michael Albinus 2019-01-09 15:24 ` Dmitry Gutov 2019-01-09 14:57 ` Dmitry Gutov 2019-01-09 23:15 ` Juri Linkov 2019-01-10 10:20 ` Dmitry Gutov 2019-01-10 21:41 ` Juri Linkov 2019-01-12 1:48 ` Dmitry Gutov 2019-01-18 3:52 ` Dmitry Gutov 2019-01-18 12:49 ` Stefan Monnier 2019-01-18 19:28 ` Dmitry Gutov 2019-01-18 21:11 ` Stefan Monnier 2019-01-18 22:53 ` Dmitry Gutov 2018-12-29 0:27 ` Dmitry Gutov 2018-12-29 17:09 ` Dmitry Gutov 2018-12-29 21:54 ` Juri Linkov 2018-12-30 23:06 ` Dmitry Gutov 2019-01-02 1:48 ` Stefan Monnier 2019-01-02 22:05 ` Juri Linkov 2019-01-03 3:44 ` Stefan Monnier 2019-01-03 20:45 ` Juri Linkov 2019-01-12 1:10 ` Making project-files the "canonical" generic, was: " Dmitry Gutov 2019-01-12 18:53 ` Making project-files the "canonical" generic Stephen Leake 2019-01-13 0:54 ` Dmitry Gutov 2019-01-15 1:14 ` Stephen Leake 2019-01-16 16:38 ` Stefan Monnier 2019-01-17 2:23 ` Dmitry Gutov 2019-01-17 13:25 ` Stefan Monnier 2019-01-18 1:00 ` Dmitry Gutov 2019-01-16 19:02 ` project--completing-read-strict breaks ada-mode project completion table Stephen Leake 2019-01-16 22:02 ` Stephen Leake 2019-01-17 23:17 ` Stephen Leake 2019-01-18 2:04 ` Dmitry Gutov 2019-01-19 3:35 ` Stephen Leake 2019-01-19 22:05 ` Dmitry Gutov 2019-01-20 19:34 ` Stephen Leake 2019-01-17 2:21 ` Dmitry Gutov 2019-01-17 13:55 ` Stefan Monnier 2019-01-17 21:35 ` John Yates 2019-01-18 2:19 ` Dmitry Gutov 2019-01-18 3:05 ` Stefan Monnier 2019-01-19 0:26 ` Dmitry Gutov 2019-01-21 19:32 ` Stephen Leake 2019-01-22 0:09 ` Dmitry Gutov 2019-02-07 1:20 ` Stephen Leake 2019-02-11 21:50 ` Stefan Monnier 2019-02-12 1:31 ` Stephen Leake 2019-02-15 15:50 ` Stephen Leake 2019-02-15 22:47 ` Stephen Leake 2019-02-15 23:38 ` Stephen Leake 2019-04-19 17:49 ` Stephen Leake 2019-05-03 0:48 ` Dmitry Gutov 2019-05-04 10:39 ` Stephen Leake 2019-05-07 18:02 ` Stephen Leake 2019-05-07 22:35 ` Dmitry Gutov 2019-05-08 1:53 ` Stefan Monnier 2019-05-14 2:14 ` Dmitry Gutov 2019-05-14 2:13 ` Dmitry Gutov 2019-02-19 17:45 ` Stefan Monnier 2019-02-20 19:58 ` Stephen Leake 2019-02-21 2:00 ` Stefan Monnier 2019-01-21 19:36 ` Stephen Leake 2019-01-22 0:20 ` Dmitry Gutov 2019-01-17 3:04 ` Making project-files the "canonical" generic Dmitry Gutov 2018-12-27 20:33 ` [Emacs-diffs] master 55ec674: * lisp/multifile.el: New file, extracted from etags.el Juri Linkov 2018-12-27 23:31 ` Dmitry Gutov 2018-12-27 23:45 ` Juri Linkov 2018-12-28 6:04 ` Dmitry Gutov 2018-12-28 18:07 ` Stefan Monnier 2018-12-29 0:31 ` Dmitry Gutov 2018-12-29 22:02 ` Juri Linkov 2018-12-30 23:13 ` Dmitry Gutov 2019-01-02 22:11 ` Juri Linkov 2019-01-02 23:23 ` Dmitry Gutov 2019-01-03 0:44 ` Juri Linkov 2019-01-03 11:52 ` Dmitry Gutov 2019-01-03 15:35 ` Stefan Monnier 2019-01-03 23:06 ` Dmitry Gutov 2019-02-07 12:23 ` Dmitry Gutov 2019-02-07 13:05 ` Stefan Monnier 2019-02-14 1:11 ` Dmitry Gutov 2019-01-03 20:57 ` Juri Linkov 2019-01-03 23:21 ` Dmitry Gutov 2019-01-05 22:12 ` Juri Linkov
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.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).