bug#44420: 28.0.50; vc-register, under vc-dir buffer, says file already registered

bug#44420: 28.0.50; vc-register, under vc-dir buffer, says file already registered

[Pankaj Jangid]

Steps:
1. Open vc-dir buffer `C-x v d'
2. Mark any changed file for further commands `m'
3. Goto an untracked file and press `i' (vc-register)
   => File already registered


In GNU Emacs 28.0.50 (build 2, x86_64-apple-darwin19.6.0, NS appkit-1894.60 Version 10.15.7 (Build 19H2))
 of 2020-11-03 built on mb2.local
Repository revision: 2d4c981b2112083e80195c0c258422e203048946
Repository branch: master
Windowing system distributor 'Apple', version 10.3.1894
System Description:  Mac OS X 10.15.7

Configured using:
 'configure LDFLAGS=-L/usr/local/opt/ruby/lib
 CPPFLAGS=-I/usr/local/opt/ruby/include
 PKG_CONFIG_PATH=:/usr/local/opt/sqlite/lib/pkgconfig:/usr/local/opt/libxml2/lib/pkgconfig:/usr/local/opt/openssl/lib/pkgconfig:/usr/local/opt/libffi/lib/pkgconfig:/usr/local/opt/ruby/lib/pkgconfig'

Configured features:
JPEG TIFF GIF PNG RSVG DBUS GLIB NOTIFY KQUEUE ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS NS MODULES THREADS JSON PDUMPER LCMS2

Important settings:
  value of $LC_CTYPE: UTF-8
  value of $LANG: en_IN.UTF-8
  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
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml easymenu mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/ns-win ns-win ucs-normalize mule-util term/common-win
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
tab-bar menu-bar rfn-eshadow isearch timer select scroll-bar mouse
jit-lock font-lock syntax facemenu 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 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 kqueue cocoa ns lcms2 multi-tty
make-network-process emacs)

Memory information:
((conses 16 52328 7020)
 (symbols 48 6730 1)
 (strings 32 17865 2285)
 (string-bytes 1 609096)
 (vectors 16 12407)
 (vector-slots 8 168897 13772)
 (floats 8 21 44)
 (intervals 56 217 0)
 (buffers 992 12))

-- 
Pankaj

bug#44420: 28.0.50; vc-register, under vc-dir buffer, says file already registered

[Andrii Kolomoiets]

Pankaj Jangid <pankaj@codeisgreat.org> writes:

> Steps:
> 1. Open vc-dir buffer `C-x v d'
> 2. Mark any changed file for further commands `m'
> 3. Goto an untracked file and press `i' (vc-register)
>    => File already registered

See docstring of the `vc-deduce-fileset`:

  If the current buffer is in `vc-dir' or Dired mode, FILESET is the
  list of marked files, or the current directory if no files are
  marked.

All commands in vc-dir buffer are applied to the marked files if any.
E.g. mark some changed file and then try to `vc-diff` (=) other file.

Including file name in the "File already registered" message may help to
understand whats going on though.

bug#44420: 28.0.50; vc-register, under vc-dir buffer, says file already registered

[Pankaj Jangid]

Andrii Kolomoiets <andreyk.mad@gmail.com> writes:

> See docstring of the `vc-deduce-fileset`:
>
>   If the current buffer is in `vc-dir' or Dired mode, FILESET is the
>   list of marked files, or the current directory if no files are
>   marked.

Got it. But the second clause `or the current directory if no files are
marked' is not accurate. Because in that case `i' (vc-register) works if
the point is on an untracked item.

Wait... is `vc-deduce-fileset' relevant for `vc-register'? I don't know.

> Including file name in the "File already registered" message may help to
> understand whats going on though.

This could be a long list. We can just say "Marked file(s) already
registered".

bug#44420: 28.0.50; vc-register, under vc-dir buffer, says file already registered

[Andrii Kolomoiets]

Pankaj Jangid <pankaj@codeisgreat.org> writes:

> Andrii Kolomoiets <andreyk.mad@gmail.com> writes:
>
>> See docstring of the `vc-deduce-fileset`:
>>
>>   If the current buffer is in `vc-dir' or Dired mode, FILESET is the
>>   list of marked files, or the current directory if no files are
>>   marked.
>
> Got it. But the second clause `or the current directory if no files are
> marked' is not accurate. Because in that case `i' (vc-register) works if
> the point is on an untracked item.

Indeed.

> Wait... is `vc-deduce-fileset' relevant for `vc-register'? I don't
> know.

Yes.  `vc-register` is calling `vc-deduce-fileset` right after `interactive`.

>> Including file name in the "File already registered" message may help to
>> understand whats going on though.
>
> This could be a long list. We can just say "Marked file(s) already
> registered".

Files in the list are checked one by one and error is returned for the
first registered file.  IMO filename will fit well in the error message.

bug#44420: 28.0.50; vc-register, under vc-dir buffer, says file already registered

[Pankaj Jangid]

Andrii Kolomoiets <andreyk.mad@gmail.com> writes:

>> Got it. But the second clause `or the current directory if no files are
>> marked' is not accurate. Because in that case `i' (vc-register) works if
>> the point is on an untracked item.
>
> Indeed.
>
>> Wait... is `vc-deduce-fileset' relevant for `vc-register'? I don't
>> know.
>
> Yes.  `vc-register` is calling `vc-deduce-fileset` right after `interactive`.

So, it's a bug.

>>> Including file name in the "File already registered" message may help to
>>> understand whats going on though.
>>
>> This could be a long list. We can just say "Marked file(s) already
>> registered".
>
> Files in the list are checked one by one and error is returned for the
> first registered file.  IMO filename will fit well in the error message.

Yes one filename will definitely fit.

bug#44420: 28.0.50; vc-register, under vc-dir buffer, says file already registered

[Dmitry Gutov]

On 05.11.2020 12:32, Pankaj Jangid wrote:
>>>> Including file name in the "File already registered" message may help to
>>>> understand whats going on though.
>>> This could be a long list. We can just say "Marked file(s) already
>>> registered".
>> Files in the list are checked one by one and error is returned for the
>> first registered file.  IMO filename will fit well in the error message.
> Yes one filename will definitely fit.

Does this work for y'all?

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 8def7da377..32664bcd25 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1391,7 +1391,7 @@ vc-register
  	(unless fname
  	  (setq fname buffer-file-name))
  	(when (vc-call-backend backend 'registered fname)
-	  (error "This file is already registered"))
+	  (error "This file is already registered: %s" fname))
  	;; Watch out for new buffers of size 0: the corresponding file
  	;; does not exist yet, even though buffer-modified-p is nil.
  	(when bname

bug#44420: 28.0.50; vc-register, under vc-dir buffer, says file already registered

[Pankaj Jangid]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Does this work for y'all?
>
> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
> index 8def7da377..32664bcd25 100644
> --- a/lisp/vc/vc.el
> +++ b/lisp/vc/vc.el
> @@ -1391,7 +1391,7 @@ vc-register
>  	(unless fname
>  	  (setq fname buffer-file-name))
>  	(when (vc-call-backend backend 'registered fname)
> -	  (error "This file is already registered"))
> +	  (error "This file is already registered: %s" fname))
>  	;; Watch out for new buffers of size 0: the corresponding file
>  	;; does not exist yet, even though buffer-modified-p is nil.
>  	(when bname

Yes. This is fine. User can then take action accordingly.

bug#44420: 28.0.50; vc-register, under vc-dir buffer, says file already registered

[Dmitry Gutov]

Version: 28.1

On 08.11.2020 04:58, Pankaj Jangid wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> Does this work for y'all?
>>
>> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
>> index 8def7da377..32664bcd25 100644
>> --- a/lisp/vc/vc.el
>> +++ b/lisp/vc/vc.el
>> @@ -1391,7 +1391,7 @@ vc-register
>>   	(unless fname
>>   	  (setq fname buffer-file-name))
>>   	(when (vc-call-backend backend 'registered fname)
>> -	  (error "This file is already registered"))
>> +	  (error "This file is already registered: %s" fname))
>>   	;; Watch out for new buffers of size 0: the corresponding file
>>   	;; does not exist yet, even though buffer-modified-p is nil.
>>   	(when bname
> 
> Yes. This is fine. User can then take action accordingly.

Very good, pushed to master.

Regarding the other concern: vc-register's docstring says:

   Register into a version control system.
If VC-FILESET is given, register the files in that fileset.
Otherwise register the current file.

so the behavior seems correct.

Let us know if there's anything still amiss, though.

bug#44420: 28.0.50; vc-register, under vc-dir buffer, says file already registered

[Andrii Kolomoiets]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Regarding the other concern: vc-register's docstring says:
>
>   Register into a version control system.
> If VC-FILESET is given, register the files in that fileset.
> Otherwise register the current file.
>
> so the behavior seems correct.
>
> Let us know if there's anything still amiss, though.

We were talking about docstring of the vc-deduce-fileset.  This part:

If the current buffer is in ‘vc-dir’ or Dired mode, FILESET is the
list of marked files, or the current directory if no files are
marked.

Maybe "current directory" can be replaced by "current item" or "item
under point".

bug#44420: 28.0.50; vc-register, under vc-dir buffer, says file already registered

[Dmitry Gutov]

On 08.11.2020 14:21, Andrii Kolomoiets wrote:
> Maybe "current directory" can be replaced by "current item" or "item
> under point".

Makes sense, thanks. Applied.