bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[Manuel Uberti]

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

This is a quick recipe:

- emacs -Q
- M-x fido-vertical-mode RET
- C-x p d
- choose a project (I picked my .emacs.d directory)
- there is a blank line between the prompt and the list of candidates
- see attached screenshot for details

In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.18, cairo 
version 1.16.0)
  of 2021-09-22 built on hathaway
Repository revision: 2a736738095c313ccef07d074aac4c5467b750e0
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12009000
System Description: Ubuntu 20.04 LTS

Configured using:
  'configure --with-harfbuzz --with-native-compilation'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
   value of $LC_MESSAGES: en_GB.UTF-8
   value of $LC_MONETARY: it_IT.UTF-8
   value of $LC_NUMERIC: it_IT.UTF-8
   value of $LC_TIME: it_IT.UTF-8
   value of $LANG: en_US.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   line-number-mode: t
   indent-tabs-mode: t
   transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg rfc6068 epg-config gnus-util rmail
rmail-loaddefs auth-source eieio eieio-core eieio-loaddefs
password-cache json map text-property-search mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
comp comp-cstr warnings rx cl-seq cl-macs cl-extra help-mode seq
byte-opt gv bytecomp byte-compile cconv rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils time-date subr-x cl-loaddefs cl-lib
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 88241 8875)
  (symbols 48 7845 0)
  (strings 32 22288 1613)
  (string-bytes 1 751603)
  (vectors 16 16329)
  (vector-slots 8 286165 15971)
  (floats 8 27 36)
  (intervals 56 208 0)
  (buffers 992 11))

-- 
Manuel Uberti
www.manueluberti.eu

[-- Attachment #2: Screenshot from 2021-09-22 10-20-34.png --]
[-- Type: image/png, Size: 13902 bytes --]

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[Dmitry Gutov]

Hi!

On 22.09.2021 11:23, Manuel Uberti wrote:
> This is a quick recipe:
> 
> - emacs -Q
> - M-x fido-vertical-mode RET
> - C-x p d
> - choose a project (I picked my .emacs.d directory)
> - there is a blank line between the prompt and the list of candidates
> - see attached screenshot for details

First of all, it's easy to fix:

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index ebd21d4b60..028de4bbdf 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -943,7 +943,8 @@ project-find-dir
           ;; `project-files-filtered', and see
           ;; https://stackoverflow.com/a/50685235/615245 for possible
           ;; implementation.
-         (all-dirs (mapcar #'file-name-directory all-files))
+         (all-dirs (delete (expand-file-name (project-root project))
+                           (mapcar #'file-name-directory all-files)))
           (dir (funcall project-read-file-name-function
                         "Dired"
                         ;; Some completion UIs show duplicates.


But should it be changed? It seems, while "" is a valid choice (which 
leads to visiting the root directory), without it in the completions set 
fido-vertical-mode doesn't easily allow you to select such option. 
Whereas the default completion UI does.

(Ivy has the same problem, except it never shows "", without or without 
the patch, so I'm fine with either).

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[Manuel Uberti]

On 22/09/21 14:01, Dmitry Gutov wrote:
> But should it be changed? It seems, while "" is a valid choice (which leads to 
> visiting the root directory), without it in the completions set 
> fido-vertical-mode doesn't easily allow you to select such option. Whereas the 
> default completion UI does.

Interesting, it seems to be just a matter of how it looks with a vertical UI 
then. With the default completion a blank makes more sense to my eyes.

However, this option leads to the root directory just like C-x p D does, IIUC. I 
guess I could live without the blank line since there is already a key binding 
for a Dired buffer in the project root.

-- 
Manuel Uberti
www.manueluberti.eu

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[Dmitry Gutov]

On 22.09.2021 16:10, Manuel Uberti wrote:
> On 22/09/21 14:01, Dmitry Gutov wrote:
>> But should it be changed? It seems, while "" is a valid choice (which 
>> leads to visiting the root directory), without it in the completions 
>> set fido-vertical-mode doesn't easily allow you to select such option. 
>> Whereas the default completion UI does.
> 
> Interesting, it seems to be just a matter of how it looks with a 
> vertical UI then. With the default completion a blank makes more sense 
> to my eyes.
> 
> However, this option leads to the root directory just like C-x p D does, 
> IIUC. I guess I could live without the blank line since there is already 
> a key binding for a Dired buffer in the project root.

That also makes a certain amount of sense.

What do other people think?

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:
>> On 22/09/21 14:01, Dmitry Gutov wrote:
>>> But should it be changed? It seems, while "" is a valid choice
>>> (which leads to visiting the root directory), without it in the
>>> completions set fido-vertical-mode doesn't easily allow you to
>>> select such option. 

M-j ?

>>> Whereas the default completion UI does.

In my tests, if I TAB TAB to see the completions with the default UI, I
also can't select the empty string that's shown in *Completions*.

Therefore I don't think this is Fido-specific.

I guess you could also put `./` there if you wanted, like M-x dired
does.  Or take it out entirely, like Manuel suggests.

João

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[Manuel Uberti]

On 22/09/21 15:36, João Távora wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
>>> On 22/09/21 14:01, Dmitry Gutov wrote:
>>>> But should it be changed? It seems, while "" is a valid choice
>>>> (which leads to visiting the root directory), without it in the
>>>> completions set fido-vertical-mode doesn't easily allow you to
>>>> select such option.
> 
> M-j ?
> 

This works and indeed makes the blank line redundant.

Since it is not possible to assume that all the vertical completion UIs out 
there have something similar to 'M-j', using './' as you suggest would make it 
easier to understand what that candidate does.

-- 
Manuel Uberti
www.manueluberti.eu

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[Juri Linkov]

>>> But should it be changed? It seems, while "" is a valid choice (which
>>> leads to visiting the root directory), without it in the completions set
>>> fido-vertical-mode doesn't easily allow you to select such
>>> option. Whereas the default completion UI does.
>> Interesting, it seems to be just a matter of how it looks with a vertical
>> UI then. With the default completion a blank makes more sense to my eyes.
>> However, this option leads to the root directory just like C-x p D does,
>> IIUC. I guess I could live without the blank line since there is already
>> a key binding for a Dired buffer in the project root.
>
> That also makes a certain amount of sense.
>
> What do other people think?

Please don't change project-find-dir, it has no problem.
The problem is in fido-vertical-mode.  It should display
an empty string with some visual indication, for example:

  <empty>
  dir1
  dir2

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[João Távora]

On Wed, Sep 22, 2021 at 5:18 PM Juri Linkov <juri@jurta.org> wrote:

> Please don't change project-find-dir, it has no problem.
> The problem is in fido-vertical-mode.

DId you miss the part where I showed that the *Completions*
buffer also displays an unselectable empty string?

Why not do what `M-x dired` does? Dired users would appreciate
the consistency, regardless of the completion UI they use.

João

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[João Távora]

On Wed, Sep 22, 2021 at 5:21 PM João Távora <joaotavora@gmail.com> wrote:
>
> On Wed, Sep 22, 2021 at 5:18 PM Juri Linkov <juri@jurta.org> wrote:
>
> > Please don't change project-find-dir, it has no problem.
> > The problem is in fido-vertical-mode.
>
> DId you miss the part where I showed that the *Completions*
> buffer also displays an unselectable empty string?
>
> Why not do what `M-x dired` does? Dired users would appreciate
> the consistency, regardless of the completion UI they use.

Regardless (I really think you should do the above), I don't object
to adding your "<empty>" idea to Icomplete and everywhere else
in Emacs that is affected by this.  At least I don't see any immediate
problems with that idea.  So the two things aren't mutually exclusive.

João

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[Juri Linkov]

>> > Please don't change project-find-dir, it has no problem.
>> > The problem is in fido-vertical-mode.
>>
>> DId you miss the part where I showed that the *Completions*
>> buffer also displays an unselectable empty string?

I confirm that the *Completions* buffer can't select it, and I've never
seen completions with an empty string.  So indeed why project-find-dir
should add it?  Maybe the problem is in project--read-file-cpd-relative?
Other completion functions allow typing RET to accept an empty string
as the default value.  Why project--read-file-cpd-relative can't do this?

>> Why not do what `M-x dired` does? Dired users would appreciate
>> the consistency, regardless of the completion UI they use.

In Dired directory names are absolute, but in project-find-dir
are relative.  Regarding your suggestion about `./`, I guess
it could be used when project--read-file-cpd-relative can't do better.

> Regardless (I really think you should do the above), I don't object
> to adding your "<empty>" idea to Icomplete and everywhere else
> in Emacs that is affected by this.  At least I don't see any immediate
> problems with that idea.  So the two things aren't mutually exclusive.

I don't know if empty strings make sense as completion candidates.

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[João Távora]

On Wed, Sep 22, 2021 at 5:52 PM Juri Linkov <juri@jurta.org> wrote:

> > Regardless (I really think you should do the above), I don't object
> > to adding your "<empty>" idea to Icomplete and everywhere else
> > in Emacs that is affected by this.  At least I don't see any immediate
> > problems with that idea.  So the two things aren't mutually exclusive.
>
> I don't know if empty strings make sense as completion candidates.

I also don't. But if some backends do produce them, maybe it does make
sense to represent them in a slightly better way.

João

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[Dmitry Gutov]

On 22.09.2021 19:14, Juri Linkov wrote:
>>>> But should it be changed? It seems, while "" is a valid choice (which
>>>> leads to visiting the root directory), without it in the completions set
>>>> fido-vertical-mode doesn't easily allow you to select such
>>>> option. Whereas the default completion UI does.
>>> Interesting, it seems to be just a matter of how it looks with a vertical
>>> UI then. With the default completion a blank makes more sense to my eyes.
>>> However, this option leads to the root directory just like C-x p D does,
>>> IIUC. I guess I could live without the blank line since there is already
>>> a key binding for a Dired buffer in the project root.
>> That also makes a certain amount of sense.
>>
>> What do other people think?
> Please don't change project-find-dir, it has no problem.
> The problem is in fido-vertical-mode.  It should display
> an empty string with some visual indication, for example:
> 
>    <empty>
>    dir1
>    dir2

While it's a possible improvement for icomplete-vertical-mode, I don't 
think it's the best UI here: showing <empty> will keep the user guessing 
what selecting this value will do.

OK, I've added './' as an option. That was a bit nontrivial because of 
the existing project-read-file-name-function abstraction.

Thanks all! Closing.

bug#50732: 28.0.50; project-find-dir: blank line in fido-vertical-mode

[Stefan Kangas]

Dmitry Gutov <dgutov@yandex.ru> writes:

> OK, I've added './' as an option. That was a bit nontrivial because of
> the existing project-read-file-name-function abstraction.

I'm a bit late to the party here, but FWIW this solution makes sense to
me as well.  Thanks!