emacs-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a pa

From: Dmitry Gutov
Subject: Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349)
Date: Tue, 30 Aug 2022 15:35:31 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 
On 30.08.2022 14:25, Eli Zaretskii wrote:

Date: Mon, 29 Aug 2022 23:36:32 +0300
Cc: larsi@gnus.org, juri@jurta.org, emacs-devel@gnu.org
From: Dmitry Gutov <dgutov@yandex.ru>


The implementation in vc-git.el does this, AFAICT:

    . creates a temporary file
    . inserts the patch into the temporary file


Note that "the patch" will usually be a subset of the current 'git diff'
output. But not always (the patch can be additionally modified by hand).


I understand.  But how is this significant?  Doesn't "git apply"
require a valid patch as well?
It's significant in that the starting conditions is that the files are already modified compared to the repository head, and the end goal is to be able to pick only some of those changes for commit. To partition a complex change into several commits, usually. Or to simply keep unfinished changes separate without resorting to 'git stash'. (**) 


    . calls "git apply --cached" on the patch in the temporary file
    . deletes the temporary file

This doesn't seem to be very different from invoking Patch on the
diffs.  (And using shell-command-on-region avoids the need to
explicitly create a temporary file.)


'patch < file' edits the working directory. 'git apply --cached' edits
the index, keeping the files themselves unchanged.


I'm not sure why is this significant.  Doesn't committing the patch
from the index eventually update the files on disk as well?


Nope.


And if
not, how is this feature useful? you get a repository where the
contents of the .git/ directory doesn't match the worktree, right?
When is this needed or useful?


See above. The two sentences near near the (**) sign.
You can also commit a patch with changes that aren't present in the files and then 'vc-revert' in them. But doing that kind of thing wasn't particularly difficult even before this feature landed IMHO. 


Personally I hope we discover some popular extension to Mercurial which
we'll be able to use in the same way as we do Git's index area here. And
then say job well done and keep the less-popular and outdated backends
unsupported.


The index thing being the problem because Git needs to have the
changes in the index before you can commit?  Or is there any other
reason?


Index is a solution, not the problem. The problem is other changes being
present on disk.


For other changes present on disk, we cam:

   . use the equivalent of the "stash" command, where the VCS supports it
   . otherwise, commit specific files by name, where the VCS supports that
   . otherwise signal an error, asking the user to clean up the tree
Like I said, I'm worried about the resulting differences in behavior. That will make it much harder for us to document the new feature properly, and the users who work with different backends from time to time are likely to stumble over those differences. 

So far using /tmp and copying seems like a better approach.


I suppose we could implement a scaled-down version of this feature as a
fallback (the one that ensures that there are no existing changes in
affected files), but I fear the difference between the backends would
trip up the users.


AFAICT, Mercurial, Bazaar, RCS, CVS, and SVN all support committing an
explicit list of files, so dirty working tree shouldn't be a problem
for them.  The first two also have a "stash"-like functionality.
Not dirty working tree. "Dirty" specific files which we want to commit certain changes in. 


But even if we initially only support this in otherwise clean trees,
it will be a huge advantage vs the current situation, where this
feature is simply not available.
Will the advantage be significant? I really have to ask, because IME applying an external patch and them committing the files as a whole never felt particularly painful before (aside from resolving any conflicts or corrupted whitespace, which the new feature doesn't help with). And 'diff-mode' has had the 'diff-apply-hunk' command for ages -- I use it often.
OTOH, the lack of support for workflow corresponding to 'git add -p' has been complained about frequently, it's one of the reasons people use Magit, and I felt it as well many times.
To improve your workflow, though, might it help to add a new command 'diff-apply-buffer'?
reply via email to
[Prev in Thread] Current Thread [Next in Thread]