unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: [Emacs-diffs] master e0ee41d: Allow customizing the display of project file names when reading
       [not found] ` <20190514021212.977E5206A2@vcs0.savannah.gnu.org>
@ 2019-05-15 17:46   ` Basil L. Contovounesios
  2019-05-15 20:31     ` Dmitry Gutov
  2019-05-15 21:11     ` Stefan Monnier
  0 siblings, 2 replies; 8+ messages in thread
From: Basil L. Contovounesios @ 2019-05-15 17:46 UTC (permalink / raw)
  To: emacs-devel; +Cc: Dmitry Gutov

dgutov@yandex.ru (Dmitry Gutov) writes:

> branch: master
> commit e0ee41d155b210327eb9c9ad5334f80ed59439f4
> Author: Dmitry Gutov <dgutov@yandex.ru>
> Commit: Dmitry Gutov <dgutov@yandex.ru>
>
>     Allow customizing the display of project file names when reading
>     
>     To hopefully resolve a long-running discussion
>     (https://lists.gnu.org/archive/html/emacs-devel/2019-05/msg00162.html).
>     
>     * lisp/progmodes/project.el (project-read-file-name-function):
>     New variable.
>     (project--read-file-absolute, project--read-file-cpd-relative):
>     New functions, possible values for the above.
>     (project-find-file-in): Use the introduced variable.
>     (project--completing-read-strict): Retain just the logic that fits
>     the name.

[...]

> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index 7c8ca15..ddb4f33 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -470,55 +464,72 @@ recognized."
>                  (project-external-roots pr))))
>      (project-find-file-in (thing-at-point 'filename) dirs pr)))
>  
> +(defcustom project-read-file-name-function #'project--read-file-cpd-relative
> +  "Function to call to read a file name from a list.
> +For the arguments list, see `project--read-file-cpd-relative'."
> +  :type '(repeat (choice (const :tag "Read with completion from relative names"
> +                                project--read-file-cpd-relative)
> +                         (const :tag "Read with completion from absolute names"
> +                                project--read-file-absolute)
> +                         (function :tag "custom function" nil))))
> +
> +(defun project--read-file-cpd-relative (prompt
> +                                        all-files &optional predicate
> +                                        hist default)
> +  (let* ((common-parent-directory
> +          (let ((common-prefix (try-completion "" all-files)))
> +            (if (> (length common-prefix) 0)
> +                (file-name-directory common-prefix))))
> +         (cpd-length (length common-parent-directory))
> +         (prompt (if (zerop cpd-length)
> +                     prompt
> +                   (concat prompt (format " in %s" common-parent-directory))))
> +         (substrings (mapcar (lambda (s) (substring s cpd-length)) all-files))
> +         (new-collection (project--file-completion-table substrings))
> +         (res (project--completing-read-strict prompt
> +                                               new-collection
> +                                               predicate
> +                                               hist default)))
> +    (concat common-parent-directory res)))

Sorry if this has been discussed before, and maybe it's just me, but I
don't think it makes sense to offer internal double hyphen functions as
alternatives for a user option, let alone to point the user to such an
internal function which lacks a docstring, "for the argument list".

Thanks,

-- 
Basil



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Emacs-diffs] master e0ee41d: Allow customizing the display of project file names when reading
  2019-05-15 17:46   ` [Emacs-diffs] master e0ee41d: Allow customizing the display of project file names when reading Basil L. Contovounesios
@ 2019-05-15 20:31     ` Dmitry Gutov
  2019-05-15 23:11       ` Basil L. Contovounesios
  2019-05-15 21:11     ` Stefan Monnier
  1 sibling, 1 reply; 8+ messages in thread
From: Dmitry Gutov @ 2019-05-15 20:31 UTC (permalink / raw)
  To: Basil L. Contovounesios, emacs-devel

On 15.05.2019 20:46, Basil L. Contovounesios wrote:
> Sorry if this has been discussed before, and maybe it's just me, but I
> don't think it makes sense to offer internal double hyphen functions as
> alternatives for a user option, let alone to point the user to such an
> internal function which lacks a docstring, "for the argument list".

No, we never discussed this in particular. So I pushed the cleanest 
version that made sense to me.

Do we not do that (put internal functions as possible values of a user 
option)?



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Emacs-diffs] master e0ee41d: Allow customizing the display of project file names when reading
  2019-05-15 17:46   ` [Emacs-diffs] master e0ee41d: Allow customizing the display of project file names when reading Basil L. Contovounesios
  2019-05-15 20:31     ` Dmitry Gutov
@ 2019-05-15 21:11     ` Stefan Monnier
  2019-05-15 23:12       ` Basil L. Contovounesios
  1 sibling, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2019-05-15 21:11 UTC (permalink / raw)
  To: emacs-devel

> Sorry if this has been discussed before, and maybe it's just me, but I
> don't think it makes sense to offer internal double hyphen functions as
> alternatives for a user option,

I think I should plead guilty, here.  Yet, I also think I agree with you.
Should I split in half?


        Stefan




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Emacs-diffs] master e0ee41d: Allow customizing the display of project file names when reading
  2019-05-15 20:31     ` Dmitry Gutov
@ 2019-05-15 23:11       ` Basil L. Contovounesios
  2019-05-16  9:51         ` Basil L. Contovounesios
  0 siblings, 1 reply; 8+ messages in thread
From: Basil L. Contovounesios @ 2019-05-15 23:11 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 15.05.2019 20:46, Basil L. Contovounesios wrote:
>> Sorry if this has been discussed before, and maybe it's just me, but I
>> don't think it makes sense to offer internal double hyphen functions as
>> alternatives for a user option, let alone to point the user to such an
>> internal function which lacks a docstring, "for the argument list".
>
> No, we never discussed this in particular. So I pushed the cleanest version that
> made sense to me.

I'm grateful for the addition, but I think user options ought to also
make sense to the user.

> Do we not do that (put internal functions as possible values of a user option)?

Sorry, the emphasis in my last message was meant to be on the lack of
any description of the user option's arglist.  If the defcustom is to
refer the user elsewhere, that place might at least have a docstring.

Though I don't understand why the user would be presented with internal
functions as options (it makes it harder to distinguish internal from
external definitions and APIs), I don't feel strongly enough about it to
argue against it.

Thanks,

-- 
Basil



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Emacs-diffs] master e0ee41d: Allow customizing the display of project file names when reading
  2019-05-15 21:11     ` Stefan Monnier
@ 2019-05-15 23:12       ` Basil L. Contovounesios
  0 siblings, 0 replies; 8+ messages in thread
From: Basil L. Contovounesios @ 2019-05-15 23:12 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Sorry if this has been discussed before, and maybe it's just me, but I
>> don't think it makes sense to offer internal double hyphen functions as
>> alternatives for a user option,
>
> I think I should plead guilty, here.  Yet, I also think I agree with you.
> Should I split in half?

That sounds painful, but a world with two Stefans sounds interesting to
say the least, so I'm split on this.

-- 
Basil



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Emacs-diffs] master e0ee41d: Allow customizing the display of project file names when reading
  2019-05-15 23:11       ` Basil L. Contovounesios
@ 2019-05-16  9:51         ` Basil L. Contovounesios
  2019-05-16 13:35           ` Dmitry Gutov
  0 siblings, 1 reply; 8+ messages in thread
From: Basil L. Contovounesios @ 2019-05-16  9:51 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Dmitry Gutov <dgutov@yandex.ru> writes:
>
>> On 15.05.2019 20:46, Basil L. Contovounesios wrote:
>>> Sorry if this has been discussed before, and maybe it's just me, but I
>>> don't think it makes sense to offer internal double hyphen functions as
>>> alternatives for a user option, let alone to point the user to such an
>>> internal function which lacks a docstring, "for the argument list".
>>
>> No, we never discussed this in particular. So I pushed the cleanest version that
>> made sense to me.
>
> I'm grateful for the addition, but I think user options ought to also
> make sense to the user.
>
>> Do we not do that (put internal functions as possible values of a user option)?
>
> Sorry, the emphasis in my last message was meant to be on the lack of
> any description of the user option's arglist.  If the defcustom is to
> refer the user elsewhere, that place might at least have a docstring.

I think even something as simple as the following would help:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: project.diff --]
[-- Type: text/x-diff, Size: 719 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index eab508af3a..cc45a71f57 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -477,6 +477,10 @@ project-read-file-name-function
 (defun project--read-file-cpd-relative (prompt
                                         all-files &optional predicate
                                         hist default)
+  "Read a file name, prompting with PROMPT.
+ALL-FILES is a list of possible file name completions.
+PREDICATE, HIST, and DEFAULT have the same meaning as in
+`completing-read'."
   (let* ((common-parent-directory
           (let ((common-prefix (try-completion "" all-files)))
             (if (> (length common-prefix) 0)

[-- Attachment #3: Type: text/plain, Size: 20 bytes --]


Thanks,

-- 
Basil

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Emacs-diffs] master e0ee41d: Allow customizing the display of project file names when reading
  2019-05-16  9:51         ` Basil L. Contovounesios
@ 2019-05-16 13:35           ` Dmitry Gutov
  2019-05-16 22:31             ` Basil L. Contovounesios
  0 siblings, 1 reply; 8+ messages in thread
From: Dmitry Gutov @ 2019-05-16 13:35 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: emacs-devel

On 16.05.2019 12:51, Basil L. Contovounesios wrote:
> I think even something as simple as the following would help:

LGTM, thanks you.

Do you have commit access?



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Emacs-diffs] master e0ee41d: Allow customizing the display of project file names when reading
  2019-05-16 13:35           ` Dmitry Gutov
@ 2019-05-16 22:31             ` Basil L. Contovounesios
  0 siblings, 0 replies; 8+ messages in thread
From: Basil L. Contovounesios @ 2019-05-16 22:31 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 16.05.2019 12:51, Basil L. Contovounesios wrote:
>> I think even something as simple as the following would help:
>
> LGTM, thanks you.
>
> Do you have commit access?

Yes, pushed to master[1].

[1: b2c0eb63dd]: Add docstring to project--read-file-cpd-relative
  2019-05-16 23:26:27 +0100
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b2c0eb63dd1f0d68de9bf7f14cc337df51617dcc

Thanks,

-- 
Basil



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-05-16 22:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20190514021209.16750.6451@vcs0.savannah.gnu.org>
     [not found] ` <20190514021212.977E5206A2@vcs0.savannah.gnu.org>
2019-05-15 17:46   ` [Emacs-diffs] master e0ee41d: Allow customizing the display of project file names when reading Basil L. Contovounesios
2019-05-15 20:31     ` Dmitry Gutov
2019-05-15 23:11       ` Basil L. Contovounesios
2019-05-16  9:51         ` Basil L. Contovounesios
2019-05-16 13:35           ` Dmitry Gutov
2019-05-16 22:31             ` Basil L. Contovounesios
2019-05-15 21:11     ` Stefan Monnier
2019-05-15 23:12       ` Basil L. Contovounesios

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).