GNU bug report logs -
#52507
[PATCH] Option for vc-delete-file to keep file on disk
Previous Next
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 52507 in the body.
You can then email your comments to 52507 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Wed, 15 Dec 2021 12:56:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Ashwin Kafle <ashwin <at> ashwink.com.np>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Wed, 15 Dec 2021 12:56:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Add a prefix argument on vc-delete-file to keep affected
file on disk and keep the current buffer intact. This option
relies on the backends to not delete files themselves.
* doc/emacs/vc1-xtra.texi: Document the change.
* lisp/vc/vc-git.el: Make git leave files on disk.
* lisp/vc/vc.el: Change vc-delete-file to accept optional prefix argument.
---
I've already signed the copyright papers.
doc/emacs/vc1-xtra.texi | 3 ++-
etc/NEWS | 4 ++++
lisp/vc/vc-git.el | 4 ++--
lisp/vc/vc.el | 20 +++++++++++---------
4 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/doc/emacs/vc1-xtra.texi b/doc/emacs/vc1-xtra.texi
index 4cd00cba6c..2cf69583b3 100644
--- a/doc/emacs/vc1-xtra.texi
+++ b/doc/emacs/vc1-xtra.texi
@@ -122,7 +122,8 @@ VC Delete/Rename
If you wish to delete a version-controlled file, use the command
@kbd{M-x vc-delete-file}. This prompts for the file name, and deletes
it via the version control system. The file is removed from the
-working tree, and in the VC Directory buffer
+working tree, and in the VC Directory buffer. If you give a prefix argument,
+the file is not deleted from disk.
@iftex
(@pxref{VC Directory Mode,,, emacs, the Emacs Manual}),
@end iftex
diff --git a/etc/NEWS b/etc/NEWS
index 8d83b2a7e3..2469060a3d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -372,6 +372,10 @@ info node. This command only works for the Emacs and Emacs Lisp manuals.
** vc
+*** 'C-x v x' accepts a prefix argument to keep file on disk
+Previously 'C-x v x' always deleted the selected file. Now if you give it
+prefix argument, it will keep the buffer and file on disk intact.
+Currently this is only implemented for vc-git.
---
*** 'C-x v v' on an unregistered file will now use the most specific backend.
Previously, if you had an SVN-covered "~/" directory, and a Git-covered
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 5c6a39aec9..69ef216529 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1569,8 +1569,8 @@ vc-git-next-revision
(or (vc-git-symbolic-commit next-rev) next-rev)))
(defun vc-git-delete-file (file)
- (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--"))
-
+ (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--cached" "--"))
+)
(defun vc-git-rename-file (old new)
(vc-git-command nil 0 (list old new) "mv" "-f" "--"))
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 64f752f248..43fc0e787e 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2970,14 +2970,13 @@ vc-transfer-file
(vc-checkin file new-backend comment (stringp comment)))))
;;;###autoload
-(defun vc-delete-file (file)
+(defun vc-delete-file (file &optional keep-file)
"Delete file and mark it as such in the version control system.
If called interactively, read FILE, defaulting to the current
-buffer's file name if it's under version control."
- (interactive (list (read-file-name "VC delete file: " nil
- (when (vc-backend buffer-file-name)
- buffer-file-name)
- t)))
+buffer's file name if it's under version control.
+If a prefix argument is given (optional argument KEEP-FILE) then
+don't delete the file from the disk"
+ (interactive "f\nP")
(setq file (expand-file-name file))
(let ((buf (get-file-buffer file))
(backend (vc-backend file)))
@@ -2999,19 +2998,22 @@ vc-delete-file
(unless (or (file-directory-p file) (null make-backup-files)
(not (file-exists-p file)))
(with-current-buffer (or buf (find-file-noselect file))
- (let ((backup-inhibited nil))
+ (let ((backup-inhibited nil)
+ ;; if you don't set this, then for some reason, the file is never brought back
+ (backup-by-copying t))
(backup-buffer))))
;; Bind `default-directory' so that the command that the backend
;; runs to remove the file is invoked in the correct context.
(let ((default-directory (file-name-directory file)))
(vc-call-backend backend 'delete-file file))
;; If the backend hasn't deleted the file itself, let's do it for him.
- (when (file-exists-p file) (delete-file file))
+ (when (and (file-exists-p file) (null keep-file))
+ (delete-file file))
;; Forget what VC knew about the file.
(vc-file-clearprops file)
;; Make sure the buffer is deleted and the *vc-dir* buffers are
;; updated after this.
- (vc-resynch-buffer file nil t)))
+ (vc-resynch-buffer file keep-file t)))
;;;###autoload
(defun vc-rename-file (old new)
--
2.34.1
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Wed, 15 Dec 2021 16:56:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 52507 <at> debbugs.gnu.org (full text, mbox):
forcemerge 52507 52508
thanks
> - (interactive (list (read-file-name "VC delete file: " nil
> - (when (vc-backend buffer-file-name)
> - buffer-file-name)
> - t)))
> + (interactive "f\nP")
I wonder why no prompt? You can add `current-prefix-arg' to the
interactive list to keep the existing prompt.
> + (let ((backup-inhibited nil)
> + ;; if you don't set this, then for some reason, the file is never brought back
> + (backup-by-copying t))
I remember having the same problem while improving `vc-rename-file'.
To solve the problem, it required adding `vc-file-clearprops'.
Maybe it could here as well?
> - (vc-resynch-buffer file nil t)))
> + (vc-resynch-buffer file keep-file t)))
It seems vc-resynch-window already uses `vc-file-clearprops'
when `keep-file' is specified, but also on some more conditions.
Forcibly Merged 52507 52508.
Request was from
Juri Linkov <juri <at> linkov.net>
to
control <at> debbugs.gnu.org
.
(Wed, 15 Dec 2021 16:56:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Wed, 15 Dec 2021 17:41:02 GMT)
Full text and
rfc822 format available.
Message #13 received at 52507 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
>> - (interactive (list (read-file-name "VC delete file: " nil
>> - (when (vc-backend buffer-file-name)
>> - buffer-file-name)
>> - t)))
>> + (interactive "f\nP")
>
> I wonder why no prompt? You can add `current-prefix-arg' to the
> interactive list to keep the existing prompt.
Thanks, I didn't knew about that.
>
>> + (let ((backup-inhibited nil)
>> + ;; if you don't set this, then for some reason, the file is never
>> brought back
>> + (backup-by-copying t))
>
> I remember having the same problem while improving `vc-rename-file'.
> To solve the problem, it required adding `vc-file-clearprops'.
> Maybe it could here as well?
There was a call to vc-file-clearprops later. I moved that earlier
before backup and that didn't seem to work.
>
>> - (vc-resynch-buffer file nil t)))
>> + (vc-resynch-buffer file keep-file t)))
>
> It seems vc-resynch-window already uses `vc-file-clearprops'
> when `keep-file' is specified, but also on some more conditions.
I'm sorry, but I didn't really understand the above. I don't really know
elisp to be honest.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Wed, 15 Dec 2021 18:10:02 GMT)
Full text and
rfc822 format available.
Message #16 received at 52507 <at> debbugs.gnu.org (full text, mbox):
>>> + (let ((backup-inhibited nil)
>>> + ;; if you don't set this, then for some reason, the file is never brought back
>>> + (backup-by-copying t))
>>
>> I remember having the same problem while improving `vc-rename-file'.
>> To solve the problem, it required adding `vc-file-clearprops'.
>> Maybe it could here as well?
>
> There was a call to vc-file-clearprops later. I moved that earlier
> before backup and that didn't seem to work.
Then another question: why backup is needed at all?
When the file is not going to be deleted, then
there is no need to make a backup copy?
>>> - (vc-resynch-buffer file nil t)))
>>> + (vc-resynch-buffer file keep-file t)))
>>
>> It seems vc-resynch-window already uses `vc-file-clearprops'
>> when `keep-file' is specified, but also on some more conditions.
>
> I'm sorry, but I didn't really understand the above. I don't really know
> elisp to be honest.
The comment before calling `vc-resynch-buffer' says it's to
make sure the buffer is deleted. But it seems the buffer
is already deleted at that point?
Also it looks that your another change is dangerous:
(defun vc-git-delete-file (file)
- (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--"))
+ (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--cached" "--")))
because it deletes the file in the staging area
that is not used by vc-git, so there is no way
to commit the deletion using vc commands.
Generally, you have a good idea, but it needs more careful handling
with the existing vc commands.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Wed, 15 Dec 2021 18:26:02 GMT)
Full text and
rfc822 format available.
Message #19 received at 52507 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Juri Linkov <juri <at> linkov.net> writes:
> Then another question: why backup is needed at all?
> When the file is not going to be deleted, then
> there is no need to make a backup copy?
There could be deletion or not depending on the value of keep-file.
Maybe the check for that can be made earlier and backup can be skipped
conditionally.
> The comment before calling `vc-resynch-buffer' says it's to
> make sure the buffer is deleted. But it seems the buffer
> is already deleted at that point?
No, the buffer is not deleted at that point. I should've changed that
comment too. vc-resynch-buffer would've deleted that buffer if the
second arg was nil.
>
> Also it looks that your another change is dangerous:
>
> (defun vc-git-delete-file (file)
> - (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--"))
> + (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--cached" "--")))
>
> because it deletes the file in the staging area
> that is not used by vc-git, so there is no way
> to commit the deletion using vc commands.
I think this check in vc-delete-file takes care of that
[Message part 2 (text/plain, inline)]
(when (eq state 'edited)
(error "Please commit or undo your changes before deleting %s" file))
[Message part 3 (text/plain, inline)]
--
I'd rather just believe that it's done by little elves running around.
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Wed, 15 Dec 2021 18:34:02 GMT)
Full text and
rfc822 format available.
Message #22 received at 52507 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Ashwin Kafle <ashwin <at> ashwink.com.np> writes:
>> Also it looks that your another change is dangerous:
>>
>> (defun vc-git-delete-file (file)
>> - (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--"))
>> + (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--cached" "--")))
>>
>> because it deletes the file in the staging area
>> that is not used by vc-git, so there is no way
>> to commit the deletion using vc commands.
>
> I think this check in vc-delete-file takes care of that
>
> (when (eq state 'edited)
> (error "Please commit or undo your changes before deleting %s" file))
Oh, you mean just that single commit can't be done by vc now. Yeah,
that seems true. Can you think of any solution for that here?
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Wed, 15 Dec 2021 23:01:02 GMT)
Full text and
rfc822 format available.
Message #25 received at 52507 <at> debbugs.gnu.org (full text, mbox):
On 15.12.2021 12:53, Ashwin Kafle wrote:
> (defun vc-git-delete-file (file)
> - (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--"))
> -
> + (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--cached" "--"))
This changes the semantics of the 'delete-file' backend action. Kind of
breaking strict compatibility with third-party backends.
I think it would be better to add an optional argument to it instead
(e.g. call it KEEP-FILE, just like the new arg to vc-delete-file; or
KEEP-ON-DISK).
Then git's implementation's signature will look like
(defun vc-git-delete-file (file &optional keep-on-disk)
and it will decide whether to add '--cached' based on that argument.
No 'delete-file' investigation will be needed as a result.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Thu, 16 Dec 2021 07:12:01 GMT)
Full text and
rfc822 format available.
Message #28 received at 52507 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 15.12.2021 12:53, Ashwin Kafle wrote:
>> (defun vc-git-delete-file (file)
>> - (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--"))
>> -
>> + (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--cached" "--"))
>
> This changes the semantics of the 'delete-file' backend action. Kind
> of breaking strict compatibility with third-party backends.
>
> I think it would be better to add an optional argument to it instead
> (e.g. call it KEEP-FILE, just like the new arg to vc-delete-file; or
> KEEP-ON-DISK).
>
> Then git's implementation's signature will look like
>
> (defun vc-git-delete-file (file &optional keep-on-disk)
>
> and it will decide whether to add '--cached' based on that argument.
>
> No 'delete-file' investigation will be needed as a result.
But this would mean that every vc-backend will have to be changed,
immediately. If we keep things as-is, Existing code either both emacs and
third-party packages will continue to work with no changes.
We can then say in NEWS that all vc backends should always leave files
in disk and let vc-delete-file handle deletion from the disk.
Also, having every vc-backend accept and check keep-on-disk will result
on a lot of duplicate code that can simply be avoided.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Thu, 16 Dec 2021 10:05:02 GMT)
Full text and
rfc822 format available.
Message #31 received at 52507 <at> debbugs.gnu.org (full text, mbox):
On 16.12.2021 10:12, Ashwin Kafle wrote:
> But this would mean that every vc-backend will have to be changed,
> immediately.
Not really: since the arg is optional, we can make sure to only add it
when it's non-nil, and otherwise call the backends with 1 argument.
> Also, having every vc-backend accept and check keep-on-disk will result
> on a lot of duplicate code that can simply be avoided.
That's a valid argument, I suppose. Depends on whether many other
backends (VCSes) know how to delete files without deleting them on disk.
OTOH, it would be handy to let those that don't declare explicitly their
inability to do that (by not supporting the second argument).
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Thu, 16 Dec 2021 11:26:02 GMT)
Full text and
rfc822 format available.
Message #34 received at 52507 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 16.12.2021 10:12, Ashwin Kafle wrote:
>> But this would mean that every vc-backend will have to be changed,
>> immediately.
>
> Not really: since the arg is optional, we can make sure to only add it
> when it's non-nil, and otherwise call the backends with 1 argument.
But then the user will get error about wrong number of arguments if the
backend doesn't support it.
>
>> Also, having every vc-backend accept and check keep-on-disk will result
>> on a lot of duplicate code that can simply be avoided.
>
> That's a valid argument, I suppose. Depends on whether many other
> backends (VCSes) know how to delete files without deleting them on
> disk.
>
> OTOH, it would be handy to let those that don't declare explicitly
> their inability to do that (by not supporting the second argument).
I think it would be better to check if the file exists after calling vc
backends. If it doesn't and keep-files is non-nil, we can restore from the
backup(which is always happening). That will make all vc backends
compatible without any change.
If other backends support deletion only from the index, then they should
do that as it preserves file-modes and such.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Thu, 16 Dec 2021 11:29:01 GMT)
Full text and
rfc822 format available.
Message #37 received at 52507 <at> debbugs.gnu.org (full text, mbox):
On 16.12.2021 14:26, Ashwin Kafle wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>
>> On 16.12.2021 10:12, Ashwin Kafle wrote:
>>> But this would mean that every vc-backend will have to be changed,
>>> immediately.
>>
>> Not really: since the arg is optional, we can make sure to only add it
>> when it's non-nil, and otherwise call the backends with 1 argument.
>
> But then the user will get error about wrong number of arguments if the
> backend doesn't support it.
That's better than having the file deleted, I think.
And our implementation can catch this particular error and show a more
humane message, too.
>>
>>> Also, having every vc-backend accept and check keep-on-disk will result
>>> on a lot of duplicate code that can simply be avoided.
>>
>> That's a valid argument, I suppose. Depends on whether many other
>> backends (VCSes) know how to delete files without deleting them on
>> disk.
>>
>> OTOH, it would be handy to let those that don't declare explicitly
>> their inability to do that (by not supporting the second argument).
>
> I think it would be better to check if the file exists after calling vc
> backends. If it doesn't and keep-files is non-nil, we can restore from the
> backup(which is always happening).
Couldn't backups be disabled?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Thu, 16 Dec 2021 14:07:01 GMT)
Full text and
rfc822 format available.
Message #40 received at 52507 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dgutov <at> yandex.ru> writes:
[...]
>> But then the user will get error about wrong number of arguments if
>> the
>> backend doesn't support it.
>
> That's better than having the file deleted, I think.
>
> And our implementation can catch this particular error and show a more
> humane message, too.
With the attached patch, the file will not be deleted when prefix is given.
[...]
>> I think it would be better to check if the file exists after calling
>> vc
>> backends. If it doesn't and keep-files is non-nil, we can restore from the
>> backup(which is always happening).
>
> Couldn't backups be disabled?
The attached patch changes make-backup-files temporarily.
[0001-Option-for-vc-delete-file-to-keep-file-on-disk.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Thu, 16 Dec 2021 17:36:03 GMT)
Full text and
rfc822 format available.
Message #43 received at 52507 <at> debbugs.gnu.org (full text, mbox):
>>> (defun vc-git-delete-file (file)
>>> - (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--"))
>>> + (vc-git-command nil 0 (vc-git--literal-pathspec file) "rm" "-f" "--cached" "--")))
>>>
>>> because it deletes the file in the staging area
>>> that is not used by vc-git, so there is no way
>>> to commit the deletion using vc commands.
>> ...
> Oh, you mean just that single commit can't be done by vc now. Yeah,
> that seems true. Can you think of any solution for that here?
--cached can't be used anyway, because vc commands doesn't use the git index.
Currently, after vc-delete-file, we have the following status in vc-dir:
./
removed file1
unregistered file1~
So the user can commit the removed file with vc-next-action.
Then after this, the user can manually rename the unregistered backup
by removing ~ from the file name.
So it seems that you want to automate the last part, i.e.
to try automatically rename the file from its backup copy
after all changes were committed?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 19 Dec 2021 23:48:01 GMT)
Full text and
rfc822 format available.
Message #46 received at 52507 <at> debbugs.gnu.org (full text, mbox):
On 16.12.2021 20:01, Juri Linkov wrote:
> --cached can't be used anyway, because vc commands doesn't use the git index.
> Currently, after vc-delete-file, we have the following status in vc-dir:
>
> ./
> removed file1
> unregistered file1~
>
> So the user can commit the removed file with vc-next-action.
That's a very good point, I didn't even consider this problem (VC not
caring about the staging area). Perhaps assuming that the full scenario
with the original patch is functional.
> Then after this, the user can manually rename the unregistered backup
> by removing ~ from the file name.
>
> So it seems that you want to automate the last part, i.e.
> to try automatically rename the file from its backup copy
> after all changes were committed?
So a "restore from backup" step indeed could be a solution for this problem.
Or alternatively, if we consider the potential feature which we've been
talking about (committing a subset of hunks from a file selectively),
its implementation should have a step which either uses a staging area,
or adds stuff to it first.
And that step could be the place to enact a change like presently
discussed (add a deletion to the staging area, and then commit it). That
deletion would either already be in the staging area (meaning we pick up
any staged changes for commit, which might be weird), or we would store
the "intent to remove with --cached" in some buffer-local variable,
which would be picked up by the new code.
The latter solution would be the "cleaner" one, but the former is one
that we could have _right now_.
On the plus side, the former also doesn't seem like it's going to
require changes in the VC API after all.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Thu, 23 Dec 2021 17:34:01 GMT)
Full text and
rfc822 format available.
Message #49 received at 52507 <at> debbugs.gnu.org (full text, mbox):
> Or alternatively, if we consider the potential feature which we've been
> talking about (committing a subset of hunks from a file selectively), its
> implementation should have a step which either uses a staging area, or adds
> stuff to it first.
>
> And that step could be the place to enact a change like presently discussed
> (add a deletion to the staging area, and then commit it). That deletion
> would either already be in the staging area (meaning we pick up any staged
> changes for commit, which might be weird), or we would store the "intent to
> remove with --cached" in some buffer-local variable, which would be picked
> up by the new code.
>
> The latter solution would be the "cleaner" one, but the former is one that
> we could have _right now_.
>
> On the plus side, the former also doesn't seem like it's going to require
> changes in the VC API after all.
The feature of committing a subset of hunks will be performed by one command
'log-edit-done' ('C-c C-c') from the *vc-log* buffer that will run git commands
`git apply --cached` followed by `git commit`.
Doing something similar could mean for example that 'C-u M-x vc-delete-file'
could immediately pop up the *vc-log* buffer waiting for a commit message,
and on 'C-c C-c' will commit only the deleted file.
I doubt that anyone might want to commit the file deletion immediately,
because file deletions usually are committed together with other changes.
But maybe git has a way to mark a file as deleted without actually deleting it?
So `git status --porcelain -z --untracked-files` could return "D" for such file
that still exists, this would be the simplest solution.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Fri, 24 Dec 2021 00:50:01 GMT)
Full text and
rfc822 format available.
Message #52 received at 52507 <at> debbugs.gnu.org (full text, mbox):
On 23.12.2021 20:20, Juri Linkov wrote:
> I doubt that anyone might want to commit the file deletion immediately,
> because file deletions usually are committed together with other changes.
Right. That's why I suggested either to have a buffer-local var to store
the "to be deleted" status, or do that in the staging area first.
> But maybe git has a way to mark a file as deleted without actually deleting it?
> So `git status --porcelain -z --untracked-files` could return "D" for such file
> that still exists, this would be the simplest solution.
'git rm --cached', used in the patch for this issue originally, is
indeed such command. Unless I misunderstood the question.
$ git rm --cached CONTRIBUTE
rm 'CONTRIBUTE'
$ git status --porcelain --untracked-files
D CONTRIBUTE
?? CONTRIBUTE
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 26 Dec 2021 14:23:01 GMT)
Full text and
rfc822 format available.
Message #55 received at 52507 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
> --cached can't be used anyway, because vc commands doesn't use the git index.
It's not --cached that is the problem. Even the unpatched
vc-delete-file effectively uses --cached because the file is removed
while backuping and never brought back for git to delete it.
Say you use the current vc-delete-file and then immediately restore it
from backup (before commiting). VC will show the file as unregistered.
I think this behavior of vc should be fixed instead.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 26 Dec 2021 14:31:02 GMT)
Full text and
rfc822 format available.
Message #58 received at 52507 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
>> Then after this, the user can manually rename the unregistered backup
>> by removing ~ from the file name.
>> So it seems that you want to automate the last part, i.e.
>> to try automatically rename the file from its backup copy
>> after all changes were committed?
>
> So a "restore from backup" step indeed could be a solution for this problem.
For current vc, restore from backup would only work after a commit which
i don't think is desirable much as you could go days without a commit
and 'C-u C-x v x' saying it will not delete the file would be wrong.
I don't know vc-internals but vc-git using git's --porcelain as juri
said is probably a much better idea.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 26 Dec 2021 15:40:01 GMT)
Full text and
rfc822 format available.
Message #61 received at 52507 <at> debbugs.gnu.org (full text, mbox):
On 26.12.2021 17:23, Ashwin Kafle wrote:
> Say you use the current vc-delete-file and then immediately restore it
> from backup (before commiting). VC will show the file as unregistered.
> I think this behavior of vc should be fixed instead.
What would you have it do instead?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 26 Dec 2021 15:51:01 GMT)
Full text and
rfc822 format available.
Message #64 received at 52507 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 26.12.2021 17:23, Ashwin Kafle wrote:
>> Say you use the current vc-delete-file and then immediately restore it
>> from backup (before commiting). VC will show the file as unregistered.
>> I think this behavior of vc should be fixed instead.
>
> What would you have it do instead?
I think a better way would be to show two files in vc-dir one saying
unregistered and one saying deleted. You mark the one saying deleted
then commit that fileset which will not be present after being commited.
That mechanism can also be used for git add -p as you can show staged
file and unsatged file separately.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 26 Dec 2021 15:59:02 GMT)
Full text and
rfc822 format available.
Message #67 received at 52507 <at> debbugs.gnu.org (full text, mbox):
On 26.12.2021 18:51, Ashwin Kafle wrote:
> Dmitry Gutov<dgutov <at> yandex.ru> writes:
>
>> On 26.12.2021 17:23, Ashwin Kafle wrote:
>>> Say you use the current vc-delete-file and then immediately restore it
>>> from backup (before commiting). VC will show the file as unregistered.
>>> I think this behavior of vc should be fixed instead.
>> What would you have it do instead?
> I think a better way would be to show two files in vc-dir one saying
> unregistered and one saying deleted. You mark the one saying deleted
> then commit that fileset which will not be present after being commited.
All right.
Well, it seems like it will add more cognitive load in the "common"
scenario -- where you end up deleting the file you said you want to delete.
And it will be a breaking change in the existing behavior/UI.
> That mechanism can also be used for git add -p as you can show staged
> file and unsatged file separately.
Perhaps it we added a different UI for staging and committing from
staging area (like in Magit), it could both be presented better and
avoid bothering the existing users who like the simpler workflow.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 26 Dec 2021 16:12:01 GMT)
Full text and
rfc822 format available.
Message #70 received at 52507 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 26.12.2021 18:51, Ashwin Kafle wrote:
>> Dmitry Gutov<dgutov <at> yandex.ru> writes:
>>
>>> On 26.12.2021 17:23, Ashwin Kafle wrote:
>>>> Say you use the current vc-delete-file and then immediately restore it
>>>> from backup (before commiting). VC will show the file as unregistered.
>>>> I think this behavior of vc should be fixed instead.
>>> What would you have it do instead?
>> I think a better way would be to show two files in vc-dir one saying
>> unregistered and one saying deleted. You mark the one saying deleted
>> then commit that fileset which will not be present after being commited.
>
> All right.
>
> Well, it seems like it will add more cognitive load in the "common"
> scenario -- where you end up deleting the file you said you want to
> delete.
>
> And it will be a breaking change in the existing behavior/UI.
If you delete from disk it behaves exactly like how it's doing right
now. The only difference should be when you delete and immediately
restore from backup and in that case, only vc-dir shows one extra file.
I don't think it brakes any existing behavior.
>
>> That mechanism can also be used for git add -p as you can show staged
>> file and unsatged file separately.
>
> Perhaps it we added a different UI for staging and committing from
> staging area (like in Magit), it could both be presented better and
> avoid bothering the existing users who like the simpler workflow.
Yeah, it's probably a bit tricky for partial adds but for complete file
deletions it should be no different at all.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 26 Dec 2021 16:30:01 GMT)
Full text and
rfc822 format available.
Message #73 received at 52507 <at> debbugs.gnu.org (full text, mbox):
On 26.12.2021 19:12, Ashwin Kafle wrote:
>> All right.
>>
>> Well, it seems like it will add more cognitive load in the "common"
>> scenario -- where you end up deleting the file you said you want to
>> delete.
>>
>> And it will be a breaking change in the existing behavior/UI.
> If you delete from disk it behaves exactly like how it's doing right
> now. The only difference should be when you delete and immediately
> restore from backup and in that case, only vc-dir shows one extra file.
> I don't think it brakes any existing behavior.
But the file would stay around, right? That would be different.
And I was referring to the VC-Dir interface which would show new
elements (additional entries for some files).
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 26 Dec 2021 17:03:01 GMT)
Full text and
rfc822 format available.
Message #76 received at 52507 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 26.12.2021 19:12, Ashwin Kafle wrote:
>>> All right.
>>>
>>> Well, it seems like it will add more cognitive load in the "common"
>>> scenario -- where you end up deleting the file you said you want to
>>> delete.
>>>
>>> And it will be a breaking change in the existing behavior/UI.
>> If you delete from disk it behaves exactly like how it's doing right
>> now. The only difference should be when you delete and immediately
>> restore from backup and in that case, only vc-dir shows one extra file.
>> I don't think it brakes any existing behavior.
>
> But the file would stay around, right? That would be different.
Only if you give vc-delete-file a prefix argument, otherwise it'll be
exactly the same. It will delete even if we use git rm --cached (because
it is checked later if the file exists anymore or not)
>
> And I was referring to the VC-Dir interface which would show new
> elements (additional entries for some files).
Only if the file is deleted in the index and has not been commited yet.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 26 Dec 2021 17:47:02 GMT)
Full text and
rfc822 format available.
Message #79 received at 52507 <at> debbugs.gnu.org (full text, mbox):
>> I doubt that anyone might want to commit the file deletion immediately,
>> because file deletions usually are committed together with other changes.
>
> Right. That's why I suggested either to have a buffer-local var to store
> the "to be deleted" status, or do that in the staging area first.
>
>> But maybe git has a way to mark a file as deleted without actually deleting it?
>> So `git status --porcelain -z --untracked-files` could return "D" for such file
>> that still exists, this would be the simplest solution.
>
> 'git rm --cached', used in the patch for this issue originally, is indeed
> such command. Unless I misunderstood the question.
>
> $ git rm --cached CONTRIBUTE
> rm 'CONTRIBUTE' $ git status --porcelain
> --untracked-files
> D CONTRIBUTE
> ?? CONTRIBUTE
Both "D " and "??" correspond to the 'unregistered' status in vc-dir
according to 'vc-git--git-status-to-vc-state':
(defun vc-git--git-status-to-vc-state (code-list)
...
('("D " "??") 'unregistered)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 26 Dec 2021 17:47:02 GMT)
Full text and
rfc822 format available.
Message #82 received at 52507 <at> debbugs.gnu.org (full text, mbox):
> Say you use the current vc-delete-file and then immediately restore it
> from backup (before commiting). VC will show the file as unregistered.
> I think this behavior of vc should be fixed instead.
Indeed, often vc causes such a state that requires to resort to
git commands like `git restore`, so vc should be improved in this regard.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Sun, 26 Dec 2021 19:14:01 GMT)
Full text and
rfc822 format available.
Message #85 received at 52507 <at> debbugs.gnu.org (full text, mbox):
On 26.12.2021 20:43, Juri Linkov wrote:
> Both "D " and "??" correspond to the 'unregistered' status in vc-dir
> according to 'vc-git--git-status-to-vc-state':
>
> (defun vc-git--git-status-to-vc-state (code-list)
> ...
> ('("D " "??") 'unregistered)
"D " corresponds to 'removed', see the third branch of (pcase code-list
...) inside vc-git--git-status-to-vc-state.
But indeed, when you pass a two-element list to
vc-git--git-status-to-vc-state, that branch is not taken, and the next
one (which you quoted) returns 'unregistered'.
That's entirely up to vc-git-state. Your new code can make a different
decision: the information is all there.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Mon, 27 Dec 2021 00:06:02 GMT)
Full text and
rfc822 format available.
Message #88 received at 52507 <at> debbugs.gnu.org (full text, mbox):
On 26.12.2021 20:03, Ashwin Kafle wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>
>> On 26.12.2021 19:12, Ashwin Kafle wrote:
>>>> All right.
>>>>
>>>> Well, it seems like it will add more cognitive load in the "common"
>>>> scenario -- where you end up deleting the file you said you want to
>>>> delete.
>>>>
>>>> And it will be a breaking change in the existing behavior/UI.
>>> If you delete from disk it behaves exactly like how it's doing right
>>> now. The only difference should be when you delete and immediately
>>> restore from backup and in that case, only vc-dir shows one extra file.
>>> I don't think it brakes any existing behavior.
>>
>> But the file would stay around, right? That would be different.
>
> Only if you give vc-delete-file a prefix argument, otherwise it'll be
> exactly the same. It will delete even if we use git rm --cached (because
> it is checked later if the file exists anymore or not)
OK, that seems to make sense. But how would we convey to the user that
that "removed" (followed by "unregistered") refers to the staging area?
Patch which would implement this in VC-Dir/Git is welcome.
And the next step would be to ensure that such deletions (which keep the
file on disk) can be committed by vc-next-action.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Mon, 27 Dec 2021 04:08:01 GMT)
Full text and
rfc822 format available.
Message #91 received at 52507 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
>>> But the file would stay around, right? That would be different.
>> Only if you give vc-delete-file a prefix argument, otherwise it'll
>> be
>> exactly the same. It will delete even if we use git rm --cached (because
>> it is checked later if the file exists anymore or not)
>
> OK, that seems to make sense. But how would we convey to the user that
> that "removed" (followed by "unregistered") refers to the staging
> area?
>
By mentioning it in the manual or perhaps in the docstring of
vc-delete-file? It should be intuitive enough once you do it.
>
> Patch which would implement this in VC-Dir/Git is welcome.
Can you please explain to me briefly how vc-dir gets this info? I will
try at it but if you don't hear from me in a week then you know what
happened ;-)
On a related note, how do you test patches? Last time i had to manually
C-M-x each and every function that was changed. Is there a way to tell
emacs to ignore byte compiled files so as a simple restart will apply
new changes? Any other way is okay with me too.
>
> And the next step would be to ensure that such deletions (which keep
> the file on disk) can be committed by vc-next-action.
>
If it shows the two files, then you can mark the one saying removed and
vc-next-action can commit it.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Mon, 27 Dec 2021 18:16:02 GMT)
Full text and
rfc822 format available.
Message #94 received at 52507 <at> debbugs.gnu.org (full text, mbox):
> Say you use the current vc-delete-file and then immediately restore it
> from backup (before commiting). VC will show the file as unregistered.
> I think this behavior of vc should be fixed instead.
Actually, to restore a deleted file there is a command 'vc-revert'.
Obviously it can't be called on a deleted file or its buffer
because it's already gone. But it can be used on a line
with the tag "removed" in the vc-dir buffer. There is
no shorter vc-dir binding to this command, so the global keybinding
'C-x v u' could be used on the vc-dir line with the removed file.
This completely reverses the deletion.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Tue, 28 Dec 2021 00:55:02 GMT)
Full text and
rfc822 format available.
Message #97 received at 52507 <at> debbugs.gnu.org (full text, mbox):
On 27.12.2021 07:08, Ashwin Kafle wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>>>> But the file would stay around, right? That would be different.
>>> Only if you give vc-delete-file a prefix argument, otherwise it'll
>>> be
>>> exactly the same. It will delete even if we use git rm --cached (because
>>> it is checked later if the file exists anymore or not)
>>
>> OK, that seems to make sense. But how would we convey to the user that
>> that "removed" (followed by "unregistered") refers to the staging
>> area?
>>
>
> By mentioning it in the manual or perhaps in the docstring of
> vc-delete-file? It should be intuitive enough once you do it.
Ideally the user interface would convey that information without
requiring the user to read the manual.
>> Patch which would implement this in VC-Dir/Git is welcome.
>
> Can you please explain to me briefly how vc-dir gets this info? I will
> try at it but if you don't hear from me in a week then you know what
> happened ;-)
It's fairly involved. There is the vc-git-dir-status-files which fetches
the status information asynchronously. But it is also updates for the
individual files when a buffer is saved, if a VC-Dir buffer is already
open (then the status comes from vc-git-state).
There is also vc-git-dir-printer which renders individual entries.
And finally, there is the ewoc.el package which stores the information
about the statuses of files which are displayed inside the VC-Dir
buffer. One should probably verify that it can show different statuses
for one file name (might be non-trivial to change, or not).
> On a related note, how do you test patches? Last time i had to manually
> C-M-x each and every function that was changed.
I either use 'M-x eval-buffer' (bound to a key, 'C-c v' in my config) to
re-evaluate the whole buffer which contains the changed code, or in some
other cases I run 'make' in the checkout which updates the .elc files,
and then restart Emacs (or rather launch a second instance for testing,
while keeping the first one running for editing).
> Is there a way to tell
> emacs to ignore byte compiled files so as a simple restart will apply
> new changes?
(setq load-prefer-newer t)
should tell Emacs to do exactly that. Though I suppose this will depend
on the order the packages are loaded -- if the edited file is loaded
before the part of your init script where this line resides, or -- even
worse -- is preloaded, this won't have the desired effect.
But otherwise this setting is very handy, and I recommend people turn it
on. Especially when developing their own packages.
>> And the next step would be to ensure that such deletions (which keep
>> the file on disk) can be committed by vc-next-action.
>>
>
> If it shows the two files, then you can mark the one saying removed and
> vc-next-action can commit it.
I mean verify that this actually works. As a UI, it sounds workable.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Thu, 08 Sep 2022 14:18:01 GMT)
Full text and
rfc822 format available.
Message #100 received at 52507 <at> debbugs.gnu.org (full text, mbox):
Reading this bug report, it's unclear what the conclusion is. I agree
that it would be useful to have a command to remove a file from the VC
without deleting it. But I don't think the proposed implementation is
safe, because it only works with git -- and in other VCs, doing `C-u M-x
vc-delete-file' would continue to delete the file, contrary to what the
doc string says.
Dmitry suggested to alter the interface, but that makes for awkward
compatibility.
Perhaps this should be a new command instead? That'd be less confusing,
I think. `M-x vc-remove-file', for instance.
Removed tag(s) patch.
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org
.
(Thu, 08 Sep 2022 14:18:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#52507
; Package
emacs
.
(Mon, 10 Mar 2025 07:01:02 GMT)
Full text and
rfc822 format available.
Message #105 received at 52507 <at> debbugs.gnu.org (full text, mbox):
close 52508
thanks
On Thu 08 Sep 2022 at 04:17pm +02, Lars Ingebrigtsen wrote:
> Reading this bug report, it's unclear what the conclusion is. I agree
> that it would be useful to have a command to remove a file from the VC
> without deleting it. But I don't think the proposed implementation is
> safe, because it only works with git -- and in other VCs, doing `C-u M-x
> vc-delete-file' would continue to delete the file, contrary to what the
> doc string says.
>
> Dmitry suggested to alter the interface, but that makes for awkward
> compatibility.
>
> Perhaps this should be a new command instead? That'd be less confusing,
> I think. `M-x vc-remove-file', for instance.
In addition, in the interim, a lot has changed in this area, because now
we have committing of individual hunks, and we had to deal with some
tricky issues relating to file addition and deletion in that context.
I think we need a fresh proposal and PoC patch to proceed any further
and I think it should probably be a new bug report. So I'm going ahead
and closing this one.
I had one general comment after reading: I think we should avoid letting
the git staging area become visible to the user as much as we possibly
can, even in Git-specific VC code.
There is the general concern of remaining compatible with past and
future VCS. But I think eschewing use of the staging area is one of
VC's selling points over Magit for people who only actually use Git.
--
Sean Whitton
bug closed, send any further explanations to
52508 <at> debbugs.gnu.org and Ashwin Kafle <ashwin <at> ashwink.com.np>
Request was from
Sean Whitton <spwhitton <at> spwhitton.name>
to
control <at> debbugs.gnu.org
.
(Mon, 10 Mar 2025 07:01:03 GMT)
Full text and
rfc822 format available.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Mon, 07 Apr 2025 11:24:05 GMT)
Full text and
rfc822 format available.
This bug report was last modified 32 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.