* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) [not found] ` <20220828194533.23A6BC00889@vcs2.savannah.gnu.org> @ 2022-08-28 21:18 ` Sean Whitton 2022-08-29 4:50 ` Manuel Uberti 2022-08-29 16:40 ` Daniel Martín 2022-08-29 14:48 ` Lars Ingebrigtsen 1 sibling, 2 replies; 95+ messages in thread From: Sean Whitton @ 2022-08-28 21:18 UTC (permalink / raw) To: emacs-devel, Juri Linkov Hello, On Sun 28 Aug 2022 at 03:45PM -04, Juri Linkov wrote: > > branch: master > commit 4803fba487d41f0817feab48b5095ef4b4940ff6 > Author: Juri Linkov <juri@linkov.net> > Commit: Juri Linkov <juri@linkov.net> > > 'C-x v v' on a diff buffer commits it as a patch (bug#52349) Very cool! Thank you! Lots of different workflows for constructing the diff are enabled by doing it in this generalised way. -- Sean Whitton ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-28 21:18 ` master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) Sean Whitton @ 2022-08-29 4:50 ` Manuel Uberti 2022-08-29 16:40 ` Daniel Martín 1 sibling, 0 replies; 95+ messages in thread From: Manuel Uberti @ 2022-08-29 4:50 UTC (permalink / raw) To: Sean Whitton, emacs-devel, Juri Linkov On 28/08/22 23:18, Sean Whitton wrote: > Hello, > > On Sun 28 Aug 2022 at 03:45PM -04, Juri Linkov wrote: > >> >> branch: master >> commit 4803fba487d41f0817feab48b5095ef4b4940ff6 >> Author: Juri Linkov <juri@linkov.net> >> Commit: Juri Linkov <juri@linkov.net> >> >> 'C-x v v' on a diff buffer commits it as a patch (bug#52349) > > Very cool! Thank you! > > Lots of different workflows for constructing the diff are enabled by > doing it in this generalised way. > Indeed, thank you for this. -- Manuel Uberti https://manueluberti.eu ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-28 21:18 ` master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) Sean Whitton 2022-08-29 4:50 ` Manuel Uberti @ 2022-08-29 16:40 ` Daniel Martín 1 sibling, 0 replies; 95+ messages in thread From: Daniel Martín @ 2022-08-29 16:40 UTC (permalink / raw) To: Sean Whitton; +Cc: emacs-devel, Juri Linkov Sean Whitton <spwhitton@spwhitton.name> writes: > Hello, > > On Sun 28 Aug 2022 at 03:45PM -04, Juri Linkov wrote: > >> >> branch: master >> commit 4803fba487d41f0817feab48b5095ef4b4940ff6 >> Author: Juri Linkov <juri@linkov.net> >> Commit: Juri Linkov <juri@linkov.net> >> >> 'C-x v v' on a diff buffer commits it as a patch (bug#52349) > > Very cool! Thank you! > > Lots of different workflows for constructing the diff are enabled by > doing it in this generalised way. +1. Thank you for working on this. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) [not found] ` <20220828194533.23A6BC00889@vcs2.savannah.gnu.org> 2022-08-28 21:18 ` master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) Sean Whitton @ 2022-08-29 14:48 ` Lars Ingebrigtsen 2022-08-29 16:46 ` Eli Zaretskii 1 sibling, 1 reply; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-08-29 14:48 UTC (permalink / raw) To: Juri Linkov; +Cc: emacs-devel Juri Linkov <juri@jurta.org> writes: > 'C-x v v' on a diff buffer commits it as a patch (bug#52349) Excellent! This is going to save me a lot of work in the future. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-29 14:48 ` Lars Ingebrigtsen @ 2022-08-29 16:46 ` Eli Zaretskii 2022-08-29 17:05 ` Dmitry Gutov 2022-08-30 13:25 ` Alfred M. Szmidt 0 siblings, 2 replies; 95+ messages in thread From: Eli Zaretskii @ 2022-08-29 16:46 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: juri, emacs-devel > From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: emacs-devel@gnu.org > Date: Mon, 29 Aug 2022 16:48:29 +0200 > > Juri Linkov <juri@jurta.org> writes: > > > 'C-x v v' on a diff buffer commits it as a patch (bug#52349) > > Excellent! This is going to save me a lot of work in the future. Bother: This is only supported for Git, which is against the spirit of VC, and definitely against the spirit of "C-x v v". Why cannot this be implemented for every other VCS? Given diffs in a buffer, it should be as simple as running the Patch utility via shell-command-on-region, then committing the results. Git has a special command for that, but we don't need a special command for other VCSes, if they don't have the equivalent of "git apply". (I'm guessing that "git apply" simply runs Patch under the hood.) Can we please implement this for other VCSes as well? ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-29 16:46 ` Eli Zaretskii @ 2022-08-29 17:05 ` Dmitry Gutov 2022-08-29 17:41 ` Eli Zaretskii 2022-08-30 7:20 ` Juri Linkov 2022-08-30 13:25 ` Alfred M. Szmidt 1 sibling, 2 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-08-29 17:05 UTC (permalink / raw) To: Eli Zaretskii, Lars Ingebrigtsen; +Cc: juri, emacs-devel On 29.08.2022 19:46, Eli Zaretskii wrote: > Why cannot this be implemented for every other VCS? It can. > Given diffs in a > buffer, it should be as simple as running the Patch utility via > shell-command-on-region, then committing the results. Git has a > special command for that, but we don't need a special command for > other VCSes, if they don't have the equivalent of "git apply". (I'm > guessing that "git apply" simply runs Patch under the hood.) No, 'git apply' puts the patch in a different place (index area), which means our implementation doesn't need to bother with moving all existing changes in the selected files somewhere else, then committing, and then restoring the previously-hidden changes. The best way to implement the latter is not obvious to me. I suppose simply copying the files to /tmp and then overwriting them back at the end might do the trick, but can cause problems if the user edits the file buffer between creating the diff and finishing the checkin. > Can we please implement this for other VCSes as well? 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. 'man hg' didn't give me enough clues, though. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-29 17:05 ` Dmitry Gutov @ 2022-08-29 17:41 ` Eli Zaretskii 2022-08-29 19:03 ` Tassilo Horn 2022-08-29 20:36 ` Dmitry Gutov 2022-08-30 7:20 ` Juri Linkov 1 sibling, 2 replies; 95+ messages in thread From: Eli Zaretskii @ 2022-08-29 17:41 UTC (permalink / raw) To: Dmitry Gutov; +Cc: larsi, juri, emacs-devel > Date: Mon, 29 Aug 2022 20:05:10 +0300 > Cc: juri@jurta.org, emacs-devel@gnu.org > From: Dmitry Gutov <dgutov@yandex.ru> > > > Given diffs in a > > buffer, it should be as simple as running the Patch utility via > > shell-command-on-region, then committing the results. Git has a > > special command for that, but we don't need a special command for > > other VCSes, if they don't have the equivalent of "git apply". (I'm > > guessing that "git apply" simply runs Patch under the hood.) > > No, 'git apply' puts the patch in a different place (index area), which > means our implementation doesn't need to bother with moving all existing > changes in the selected files somewhere else, then committing, and then > restoring the previously-hidden changes. I don't follow, sorry. The implementation in vc-git.el does this, AFAICT: . creates a temporary file . inserts the patch into the temporary file . 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.) > 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? IOW, why cannot we simply patch the files, and then run the equivalent of vc-checkin? ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-29 17:41 ` Eli Zaretskii @ 2022-08-29 19:03 ` Tassilo Horn 2022-08-29 19:31 ` Eli Zaretskii 2022-08-29 20:36 ` Dmitry Gutov 1 sibling, 1 reply; 95+ messages in thread From: Tassilo Horn @ 2022-08-29 19:03 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Dmitry Gutov, larsi, juri, emacs-devel Eli Zaretskii <eliz@gnu.org> writes: >> No, 'git apply' puts the patch in a different place (index area), >> which means our implementation doesn't need to bother with moving all >> existing changes in the selected files somewhere else, then >> committing, and then restoring the previously-hidden changes. > > I don't follow, sorry. > > The implementation in vc-git.el does this, AFAICT: > > . creates a temporary file > . inserts the patch into the temporary file > . 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. I think the difference is that "git apply --cached foo.patch" exits non-zero if the index isn't empty and vc-git-checkin errors when it's not. That ensures the commit will include only the changes in the patch and nothing else. I guess other VCS impls could just test if the checkout is completely unmodified and signal an error otherwise. That would be a bit more restrictive but still useful. >> 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? I think the git benefit is that you can have a completely dirty checkout (hundreds of modified files) but the command will still work fine as long as you haven't staged other changes yet. > IOW, why cannot we simply patch the files, and then run the equivalent > of vc-checkin? See above, I think we can at least after a "is the checkout clean?" check. Bye, Tassilo ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-29 19:03 ` Tassilo Horn @ 2022-08-29 19:31 ` Eli Zaretskii 0 siblings, 0 replies; 95+ messages in thread From: Eli Zaretskii @ 2022-08-29 19:31 UTC (permalink / raw) To: Tassilo Horn; +Cc: dgutov, larsi, juri, emacs-devel > From: Tassilo Horn <tsdh@gnu.org> > Cc: Dmitry Gutov <dgutov@yandex.ru>, larsi@gnus.org, juri@jurta.org, > emacs-devel@gnu.org > Date: Mon, 29 Aug 2022 21:03:50 +0200 > > I think the difference is that "git apply --cached foo.patch" exits > non-zero if the index isn't empty and vc-git-checkin errors when it's > not. That ensures the commit will include only the changes in the patch > and nothing else. > > I guess other VCS impls could just test if the checkout is completely > unmodified and signal an error otherwise. Either that, or compare the list of modified files with the list of patched files produced by Patch. > That would be a bit more restrictive but still useful. Yes, indeed. > >> 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? > > I think the git benefit is that you can have a completely dirty checkout > (hundreds of modified files) but the command will still work fine as > long as you haven't staged other changes yet. It is perfectly fine to have the Git implementation be more flexible and convenient than with other VCSes, which don't support the same granularity/precision. It would still be much better than having no implementation at all for the other VCSes. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-29 17:41 ` Eli Zaretskii 2022-08-29 19:03 ` Tassilo Horn @ 2022-08-29 20:36 ` Dmitry Gutov 2022-08-30 11:25 ` Eli Zaretskii 1 sibling, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-29 20:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, juri, emacs-devel On 29.08.2022 20:41, Eli Zaretskii wrote: >> Date: Mon, 29 Aug 2022 20:05:10 +0300 >> Cc: juri@jurta.org, emacs-devel@gnu.org >> From: Dmitry Gutov <dgutov@yandex.ru> >> >>> Given diffs in a >>> buffer, it should be as simple as running the Patch utility via >>> shell-command-on-region, then committing the results. Git has a >>> special command for that, but we don't need a special command for >>> other VCSes, if they don't have the equivalent of "git apply". (I'm >>> guessing that "git apply" simply runs Patch under the hood.) >> >> No, 'git apply' puts the patch in a different place (index area), which >> means our implementation doesn't need to bother with moving all existing >> changes in the selected files somewhere else, then committing, and then >> restoring the previously-hidden changes. > > I don't follow, sorry. > > 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). This feature is a replacement for the popular workflow 'git add -p; git commit', not for simply committing a patch, which is pretty easy to do already. Though the beauty of the current approach with Git is that it supports either. > . 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. >> 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. > IOW, why cannot we simply patch the files, and then run the equivalent > of vc-checkin? 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. So we should start with trying to implement the "full" version first: for individual backends or (harder) in general. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-29 20:36 ` Dmitry Gutov @ 2022-08-30 11:25 ` Eli Zaretskii 2022-08-30 12:35 ` Dmitry Gutov 0 siblings, 1 reply; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 11:25 UTC (permalink / raw) To: Dmitry Gutov; +Cc: larsi, juri, emacs-devel > 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? > This feature is a replacement for the popular workflow 'git add -p; git > commit', not for simply committing a patch, which is pretty easy to do > already. Yes, I understand that. > > . 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? 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? > >> 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 > 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. 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. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 11:25 ` Eli Zaretskii @ 2022-08-30 12:35 ` Dmitry Gutov 2022-08-30 13:01 ` Eli Zaretskii 2022-08-30 16:47 ` Juri Linkov 0 siblings, 2 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 12:35 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, juri, emacs-devel 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'? ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 12:35 ` Dmitry Gutov @ 2022-08-30 13:01 ` Eli Zaretskii 2022-08-30 13:52 ` Dmitry Gutov 2022-08-30 16:47 ` Juri Linkov 1 sibling, 1 reply; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 13:01 UTC (permalink / raw) To: Dmitry Gutov; +Cc: larsi, juri, emacs-devel > Date: Tue, 30 Aug 2022 15:35:31 +0300 > Cc: larsi@gnus.org, juri@jurta.org, emacs-devel@gnu.org > From: Dmitry Gutov <dgutov@yandex.ru> > > >> 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. Ouch! Then this feature will be useless for me, and I'm sorry I wasted everyone's time based on the description in NEWS, which doesn't make a point of emphasizing this basic assumption. > 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. I cannot agree that this feature is a reasonable solution for the "git add -p" workflow. It is not interactive enough for that. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 13:01 ` Eli Zaretskii @ 2022-08-30 13:52 ` Dmitry Gutov 2022-08-30 16:20 ` Eli Zaretskii 0 siblings, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 13:52 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, juri, emacs-devel On 30.08.2022 16:01, Eli Zaretskii wrote: >> Date: Tue, 30 Aug 2022 15:35:31 +0300 >> Cc: larsi@gnus.org, juri@jurta.org, emacs-devel@gnu.org >> From: Dmitry Gutov <dgutov@yandex.ru> >> >>>> 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. > > Ouch! Then this feature will be useless for me, and I'm sorry I > wasted everyone's time based on the description in NEWS, which doesn't > make a point of emphasizing this basic assumption. Perhaps we could make it more useful for you as well, e.g. by applying all the changes from the patch first when called with 'C-u'. I guess the problem is that the first step is not so easily automated: at least in my experience 'diff-apply-hunk' will either fail to find the files to patch (and prompt me to find them manually), or fail because of whitespace problems, or because it was made against some previous version of the code, and now has conflicts. And after I resolve all of this by hand, the files now have all the changes applied, and we're in the situation where 'C-x v D' followed by the new 'C-x v v' in diff-mode works just fine. But I suppose if your patches apply cleanly most of the time, it can be less of a problem. >> 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. > > I cannot agree that this feature is a reasonable solution for the > "git add -p" workflow. It is not interactive enough for that. It's... a more power-user approach to the same, with a different UI. Experience shows that this workflow can work as an alternative, evidenced by the userbase of the commit-patch package. diff-mode provides commands to delete hunks, split them, and even allows one to edit individual characters (something that 'git add -p' doesn't provide). So I'd argue this way might even be more powerful. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 13:52 ` Dmitry Gutov @ 2022-08-30 16:20 ` Eli Zaretskii 2022-08-30 17:33 ` Dmitry Gutov 0 siblings, 1 reply; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 16:20 UTC (permalink / raw) To: Dmitry Gutov; +Cc: larsi, juri, emacs-devel > Date: Tue, 30 Aug 2022 16:52:10 +0300 > Cc: larsi@gnus.org, juri@jurta.org, emacs-devel@gnu.org > From: Dmitry Gutov <dgutov@yandex.ru> > > >> 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. > > > > Ouch! Then this feature will be useless for me, and I'm sorry I > > wasted everyone's time based on the description in NEWS, which doesn't > > make a point of emphasizing this basic assumption. > > Perhaps we could make it more useful for you as well, e.g. by applying > all the changes from the patch first when called with 'C-u'. Alas, C-u already has a meaning with "C-x v v". > But I suppose if your patches apply cleanly most of the time, it can be > less of a problem. They do, when I choose them not to be rejected ;-) > diff-mode provides commands to delete hunks, split them, and even allows > one to edit individual characters (something that 'git add -p' doesn't > provide). So I'd argue this way might even be more powerful. From my POV, a better UI would be to enter diff-mode _after_ the user invoked VC to commit the changes, but asked specifically to commit them selectively. Not the other way around. IOW, the process of generating the diffs buffer should be part of the command execution, not a prerequisite for it. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 16:20 ` Eli Zaretskii @ 2022-08-30 17:33 ` Dmitry Gutov 0 siblings, 0 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 17:33 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, juri, emacs-devel On 30.08.2022 19:20, Eli Zaretskii wrote: >> Date: Tue, 30 Aug 2022 16:52:10 +0300 >> Cc: larsi@gnus.org, juri@jurta.org, emacs-devel@gnu.org >> From: Dmitry Gutov <dgutov@yandex.ru> >> >>>> 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. >>> >>> Ouch! Then this feature will be useless for me, and I'm sorry I >>> wasted everyone's time based on the description in NEWS, which doesn't >>> make a point of emphasizing this basic assumption. >> >> Perhaps we could make it more useful for you as well, e.g. by applying >> all the changes from the patch first when called with 'C-u'. > > Alas, C-u already has a meaning with "C-x v v". For better or worse, it doesn't seem to do anything in diff-mode buffers. But we could also use 'C-0'. Or something similar. >> But I suppose if your patches apply cleanly most of the time, it can be >> less of a problem. > > They do, when I choose them not to be rejected ;-) Admirable. >> diff-mode provides commands to delete hunks, split them, and even allows >> one to edit individual characters (something that 'git add -p' doesn't >> provide). So I'd argue this way might even be more powerful. > > From my POV, a better UI would be to enter diff-mode _after_ the user > invoked VC to commit the changes, but asked specifically to commit > them selectively. Not the other way around. IOW, the process of > generating the diffs buffer should be part of the command execution, > not a prerequisite for it. I suppose this UI could be implemented on top of the current behavior as well. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 12:35 ` Dmitry Gutov 2022-08-30 13:01 ` Eli Zaretskii @ 2022-08-30 16:47 ` Juri Linkov 2022-08-30 17:05 ` Eli Zaretskii 1 sibling, 1 reply; 95+ messages in thread From: Juri Linkov @ 2022-08-30 16:47 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Eli Zaretskii, larsi, emacs-devel >> 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). Indeed, just 'M-! git apply' before using 'vc-checkin-patch' will do the job in case of an external patch. > 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'? There is 'ediff-patch-buffer' that does this interactively like 'git add -p'. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 16:47 ` Juri Linkov @ 2022-08-30 17:05 ` Eli Zaretskii 2022-08-30 17:11 ` Dmitry Gutov 2022-08-30 17:18 ` Juri Linkov 0 siblings, 2 replies; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 17:05 UTC (permalink / raw) To: Juri Linkov; +Cc: dgutov, larsi, emacs-devel > From: Juri Linkov <juri@jurta.org> > Cc: Eli Zaretskii <eliz@gnu.org>, larsi@gnus.org, emacs-devel@gnu.org > Date: Tue, 30 Aug 2022 19:47:35 +0300 > > >> 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). > > Indeed, just 'M-! git apply' before using 'vc-checkin-patch' will do the job > in case of an external patch. Unless some of the hunks are wrong or produce conflicts or are rejected due to 1001 reasons. Which is exactly the use case I thought this enabled. Sorry for misreading. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 17:05 ` Eli Zaretskii @ 2022-08-30 17:11 ` Dmitry Gutov 2022-08-30 17:37 ` Eli Zaretskii 2022-08-30 17:18 ` Juri Linkov 1 sibling, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 17:11 UTC (permalink / raw) To: Eli Zaretskii, Juri Linkov; +Cc: larsi, emacs-devel On 30.08.2022 20:05, Eli Zaretskii wrote: >> Indeed, just 'M-! git apply' before using 'vc-checkin-patch' will do the job >> in case of an external patch. > Unless some of the hunks are wrong or produce conflicts or are > rejected due to 1001 reasons. Which is exactly the use case I thought > this enabled. Sorry for misreading. Not sure how we could possibly automate that problem away. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 17:11 ` Dmitry Gutov @ 2022-08-30 17:37 ` Eli Zaretskii 2022-08-30 17:39 ` Dmitry Gutov 0 siblings, 1 reply; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 17:37 UTC (permalink / raw) To: Dmitry Gutov; +Cc: juri, larsi, emacs-devel > Date: Tue, 30 Aug 2022 20:11:52 +0300 > Cc: larsi@gnus.org, emacs-devel@gnu.org > From: Dmitry Gutov <dgutov@yandex.ru> > > On 30.08.2022 20:05, Eli Zaretskii wrote: > >> Indeed, just 'M-! git apply' before using 'vc-checkin-patch' will do the job > >> in case of an external patch. > > Unless some of the hunks are wrong or produce conflicts or are > > rejected due to 1001 reasons. Which is exactly the use case I thought > > this enabled. Sorry for misreading. > > Not sure how we could possibly automate that problem away. You don't need to. Aborting a botched patch is easy; once the problematic hunks are known (they are announced by Patch or by "git apply"), you use this facility to filter them out, and patch/commit again. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 17:37 ` Eli Zaretskii @ 2022-08-30 17:39 ` Dmitry Gutov 0 siblings, 0 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 17:39 UTC (permalink / raw) To: Eli Zaretskii; +Cc: juri, larsi, emacs-devel On 30.08.2022 20:37, Eli Zaretskii wrote: >> Date: Tue, 30 Aug 2022 20:11:52 +0300 >> Cc:larsi@gnus.org,emacs-devel@gnu.org >> From: Dmitry Gutov<dgutov@yandex.ru> >> >> On 30.08.2022 20:05, Eli Zaretskii wrote: >>>> Indeed, just 'M-! git apply' before using 'vc-checkin-patch' will do the job >>>> in case of an external patch. >>> Unless some of the hunks are wrong or produce conflicts or are >>> rejected due to 1001 reasons. Which is exactly the use case I thought >>> this enabled. Sorry for misreading. >> Not sure how we could possibly automate that problem away. > You don't need to. Aborting a botched patch is easy; once the > problematic hunks are known (they are announced by Patch or by > "git apply"), you use this facility to filter them out, and > patch/commit again. But filtering those out is not what one usually wants to do. It's more common to want to install all the included changes, even if some might require fixing. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 17:05 ` Eli Zaretskii 2022-08-30 17:11 ` Dmitry Gutov @ 2022-08-30 17:18 ` Juri Linkov 1 sibling, 0 replies; 95+ messages in thread From: Juri Linkov @ 2022-08-30 17:18 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dgutov, larsi, emacs-devel >> >> 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). >> >> Indeed, just 'M-! git apply' before using 'vc-checkin-patch' will do the job >> in case of an external patch. > > Unless some of the hunks are wrong or produce conflicts or are > rejected due to 1001 reasons. Which is exactly the use case I thought > this enabled. Sorry for misreading. 'vc-git-checkin-patch' is conflict-free. It bypasses the working tree. OTOH, applying a patch to the working tree often requires manual resolution. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-29 17:05 ` Dmitry Gutov 2022-08-29 17:41 ` Eli Zaretskii @ 2022-08-30 7:20 ` Juri Linkov 2022-08-30 11:54 ` Eli Zaretskii ` (2 more replies) 1 sibling, 3 replies; 95+ messages in thread From: Juri Linkov @ 2022-08-30 7:20 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Eli Zaretskii, Lars Ingebrigtsen, emacs-devel >> Can we please implement this for other VCSes as well? > > 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. > > 'man hg' didn't give me enough clues, though. I am unfamiliar with Mercurial, but looking at https://www.mercurial-scm.org/wiki/GitConcepts#Git.27s_staging_area where the table says that index/staging area is not necessary in Mercurial and suggests to use shelve instead that corresponds to Git's stash, it would be possible to do the same with Hg as we first tried to do with Git using stash: put changes to stash/shelve, commit and restore the previous state. In older VCSes that have neither index/staging area nor stash/shelve, working area should be cleared by temporarily moving changed files somewhere. Anyway, the main point is that adepts of any existing VCS or any future VCS can easily adapt their VCSes for this feature by just implementing the backend call 'checkin-patch'. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 7:20 ` Juri Linkov @ 2022-08-30 11:54 ` Eli Zaretskii 2022-08-30 14:49 ` Sean Whitton 2022-08-30 11:56 ` Eli Zaretskii 2022-08-30 12:03 ` Dmitry Gutov 2 siblings, 1 reply; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 11:54 UTC (permalink / raw) To: Juri Linkov; +Cc: dgutov, larsi, emacs-devel > From: Juri Linkov <juri@jurta.org> > Cc: Eli Zaretskii <eliz@gnu.org>, Lars Ingebrigtsen <larsi@gnus.org>, > emacs-devel@gnu.org > Date: Tue, 30 Aug 2022 10:20:10 +0300 > > Anyway, the main point is that adepts of any existing VCS or any future > VCS can easily adapt their VCSes for this feature by just implementing > the backend call 'checkin-patch'. I very much hope this is not going to be the official policy of VC development in Emacs. VC should IMO remain VCS-agnostic in its most important features, and "C-x v v" is definitely such a feature of VC. That's basically the only real raison d'être of VC nowadays, because otherwise let's just obsolete VC and switch to Magit. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 11:54 ` Eli Zaretskii @ 2022-08-30 14:49 ` Sean Whitton 2022-08-30 16:39 ` Juri Linkov ` (2 more replies) 0 siblings, 3 replies; 95+ messages in thread From: Sean Whitton @ 2022-08-30 14:49 UTC (permalink / raw) To: Eli Zaretskii, Juri Linkov, dgutov, larsi, emacs-devel Hello, On Tue 30 Aug 2022 at 02:54PM +03, Eli Zaretskii wrote: >> From: Juri Linkov <juri@jurta.org> >> Cc: Eli Zaretskii <eliz@gnu.org>, Lars Ingebrigtsen <larsi@gnus.org>, >> emacs-devel@gnu.org >> Date: Tue, 30 Aug 2022 10:20:10 +0300 >> >> Anyway, the main point is that adepts of any existing VCS or any future >> VCS can easily adapt their VCSes for this feature by just implementing >> the backend call 'checkin-patch'. > > I very much hope this is not going to be the official policy of VC > development in Emacs. VC should IMO remain VCS-agnostic in its most > important features, and "C-x v v" is definitely such a feature of VC. Surely leaving room for things to be implemented for other VCs means that it remains VC-agnostic. With e.g. Magit there isn't even that room. > That's basically the only real raison d'être of VC nowadays, because > otherwise let's just obsolete VC and switch to Magit. VC is far more flexible and Emacs-y than Magit in how it handles buffers. E.g. in Magit if you C-x x u a log buffer, because you want to log something else at the same time, Magit will re-use the first log buffer anyway. So for those of us who like it when things are more buffer-oriented, it remains a good alternative. -- Sean Whitton ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 14:49 ` Sean Whitton @ 2022-08-30 16:39 ` Juri Linkov 2022-08-30 16:45 ` Eli Zaretskii 2022-08-31 2:38 ` Richard Stallman 2 siblings, 0 replies; 95+ messages in thread From: Juri Linkov @ 2022-08-30 16:39 UTC (permalink / raw) To: Sean Whitton; +Cc: Eli Zaretskii, dgutov, larsi, emacs-devel > VC is far more flexible and Emacs-y than Magit in how it handles > buffers. E.g. in Magit if you C-x x u a log buffer, because you want to > log something else at the same time, Magit will re-use the first log > buffer anyway. So for those of us who like it when things are more > buffer-oriented, it remains a good alternative. Indeed, everything works fine in VC with such customization: (add-hook 'diff-mode-hook 'rename-uniquely) (add-hook 'log-view-mode-hook 'rename-uniquely) (add-hook 'log-edit-mode-hook 'rename-uniquely) ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 14:49 ` Sean Whitton 2022-08-30 16:39 ` Juri Linkov @ 2022-08-30 16:45 ` Eli Zaretskii 2022-08-31 2:38 ` Richard Stallman 2022-08-31 15:48 ` Jonas Bernoulli 2022-08-31 2:38 ` Richard Stallman 2 siblings, 2 replies; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 16:45 UTC (permalink / raw) To: Sean Whitton; +Cc: juri, dgutov, larsi, emacs-devel > From: Sean Whitton <spwhitton@spwhitton.name> > Date: Tue, 30 Aug 2022 07:49:56 -0700 > > >> Anyway, the main point is that adepts of any existing VCS or any future > >> VCS can easily adapt their VCSes for this feature by just implementing > >> the backend call 'checkin-patch'. > > > > I very much hope this is not going to be the official policy of VC > > development in Emacs. VC should IMO remain VCS-agnostic in its most > > important features, and "C-x v v" is definitely such a feature of VC. > > Surely leaving room for things to be implemented for other VCs means > that it remains VC-agnostic. With e.g. Magit there isn't even that room. Yeah, and then let's "leave room" for adepts of documentation to add it if they feel like it, and for adepts of Lisp style to fix the style of the code someone else installed, and for adepts of clean design and implementation to clean up after others, etc. etc. We have some conventions and some standards in Emacs development, so I'm surprised to hear that some of them are left to "adepts" to uphold. If we go that way, there are good chances that some things will never happen, because no adepts were in supply, or because we forgot, or for any number of other reasons. > > That's basically the only real raison d'être of VC nowadays, because > > otherwise let's just obsolete VC and switch to Magit. > > VC is far more flexible and Emacs-y than Magit in how it handles > buffers. I'm sure frequent users of Magit disagree. Of course, that's not why I mentioned Magit: I mentioned it because if we want to cater to a single VCS, a very good solution for that already exists. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 16:45 ` Eli Zaretskii @ 2022-08-31 2:38 ` Richard Stallman 2022-08-31 15:48 ` Jonas Bernoulli 1 sibling, 0 replies; 95+ messages in thread From: Richard Stallman @ 2022-08-31 2:38 UTC (permalink / raw) To: Eli Zaretskii; +Cc: spwhitton, juri, dgutov, larsi, emacs-devel [[[ To any NSA and FBI agents reading my email: please consider ]]] [[[ whether defending the US Constitution against all enemies, ]]] [[[ foreign or domestic, requires you to follow Snowden's example. ]]] > > Surely leaving room for things to be implemented for other VCs means > > that it remains VC-agnostic. With e.g. Magit there isn't even that room. > Yeah, and then let's "leave room" for adepts of documentation to add > it if they feel like it, and for adepts of Lisp style to fix the style > of the code someone else installed, and for adepts of clean design and > implementation to clean up after others, etc. etc. Hear, hear! Finishing a change fully is a necessary part of starting it. -- Dr Richard Stallman (https://stallman.org) Chief GNUisance of the GNU Project (https://gnu.org) Founder, Free Software Foundation (https://fsf.org) Internet Hall-of-Famer (https://internethalloffame.org) ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 16:45 ` Eli Zaretskii 2022-08-31 2:38 ` Richard Stallman @ 2022-08-31 15:48 ` Jonas Bernoulli 1 sibling, 0 replies; 95+ messages in thread From: Jonas Bernoulli @ 2022-08-31 15:48 UTC (permalink / raw) To: Eli Zaretskii, Sean Whitton; +Cc: juri, dgutov, larsi, emacs-devel Eli Zaretskii <eliz@gnu.org> writes: >> VC is far more flexible and Emacs-y than Magit in how it handles >> buffers. > > I'm sure frequent users of Magit disagree. Maybe, but as its maintainer, I increasingly agree. ;) A quick hack to make it use more buffers: (add-hook 'magit-create-buffer-hook 'magit-create-buffer-lock) (defun magit-create-buffer-lock () (and (magit-buffer-value) (magit-toggle-buffer-lock))) (I wrote that just now and didn't test much. Things might blow up.) ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 14:49 ` Sean Whitton 2022-08-30 16:39 ` Juri Linkov 2022-08-30 16:45 ` Eli Zaretskii @ 2022-08-31 2:38 ` Richard Stallman 2022-09-01 0:32 ` Sean Whitton 2 siblings, 1 reply; 95+ messages in thread From: Richard Stallman @ 2022-08-31 2:38 UTC (permalink / raw) To: Sean Whitton; +Cc: eliz, juri, dgutov, larsi, emacs-devel [[[ To any NSA and FBI agents reading my email: please consider ]]] [[[ whether defending the US Constitution against all enemies, ]]] [[[ foreign or domestic, requires you to follow Snowden's example. ]]] > VC is far more flexible and Emacs-y than Magit in how it handles > buffers. E.g. in Magit if you C-x x u a log buffer, because you want to > log something else at the same time, Magit will re-use the first log > buffer anyway. Isn't that a bug in Magit? It could be fixed to handle special buffers in the usual Emacs fashion that other packages use. -- Dr Richard Stallman (https://stallman.org) Chief GNUisance of the GNU Project (https://gnu.org) Founder, Free Software Foundation (https://fsf.org) Internet Hall-of-Famer (https://internethalloffame.org) ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 2:38 ` Richard Stallman @ 2022-09-01 0:32 ` Sean Whitton 2022-09-01 0:33 ` Sean Whitton 0 siblings, 1 reply; 95+ messages in thread From: Sean Whitton @ 2022-09-01 0:32 UTC (permalink / raw) To: Richard Stallman, eliz, juri, dgutov, larsi, emacs-devel Hello, On Tue 30 Aug 2022 at 10:38PM -04, Richard Stallman wrote: > [[[ To any NSA and FBI agents reading my email: please consider ]]] > [[[ whether defending the US Constitution against all enemies, ]]] > [[[ foreign or domestic, requires you to follow Snowden's example. ]]] > > > VC is far more flexible and Emacs-y than Magit in how it handles > > buffers. E.g. in Magit if you C-x x u a log buffer, because you want to > > log something else at the same time, Magit will re-use the first log > > buffer anyway. > > Isn't that a bug in Magit? It could be fixed > to handle special buffers in the usual Emacs fashion that other packages use. It may well be a deliberate UI choice. -- Sean Whitton ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-01 0:32 ` Sean Whitton @ 2022-09-01 0:33 ` Sean Whitton 0 siblings, 0 replies; 95+ messages in thread From: Sean Whitton @ 2022-09-01 0:33 UTC (permalink / raw) To: Richard Stallman, eliz, juri, dgutov, larsi, emacs-devel Hello, On Wed 31 Aug 2022 at 05:32PM -07, Sean Whitton wrote: > Hello, > > On Tue 30 Aug 2022 at 10:38PM -04, Richard Stallman wrote: > >> [[[ To any NSA and FBI agents reading my email: please consider ]]] >> [[[ whether defending the US Constitution against all enemies, ]]] >> [[[ foreign or domestic, requires you to follow Snowden's example. ]]] >> >> > VC is far more flexible and Emacs-y than Magit in how it handles >> > buffers. E.g. in Magit if you C-x x u a log buffer, because you want to >> > log something else at the same time, Magit will re-use the first log >> > buffer anyway. >> >> Isn't that a bug in Magit? It could be fixed >> to handle special buffers in the usual Emacs fashion that other packages use. > > It may well be a deliberate UI choice. Ah, Jonas says downthread it might indeed be under review, apologies. -- Sean Whitton ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 7:20 ` Juri Linkov 2022-08-30 11:54 ` Eli Zaretskii @ 2022-08-30 11:56 ` Eli Zaretskii 2022-08-30 12:03 ` Dmitry Gutov 2 siblings, 0 replies; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 11:56 UTC (permalink / raw) To: Juri Linkov; +Cc: dgutov, larsi, emacs-devel > From: Juri Linkov <juri@jurta.org> > Cc: Eli Zaretskii <eliz@gnu.org>, Lars Ingebrigtsen <larsi@gnus.org>, > emacs-devel@gnu.org > Date: Tue, 30 Aug 2022 10:20:10 +0300 > > In older VCSes that have neither index/staging area nor > stash/shelve, working area should be cleared by temporarily moving > changed files somewhere. There should be no need to move anything, as those older VCSes (and the new ones as well) can accept a list of files to commit in their commit command. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 7:20 ` Juri Linkov 2022-08-30 11:54 ` Eli Zaretskii 2022-08-30 11:56 ` Eli Zaretskii @ 2022-08-30 12:03 ` Dmitry Gutov 2022-09-01 20:13 ` Dmitry Gutov 2 siblings, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 12:03 UTC (permalink / raw) To: Juri Linkov; +Cc: Eli Zaretskii, Lars Ingebrigtsen, emacs-devel On 30.08.2022 10:20, Juri Linkov wrote: > I am unfamiliar with Mercurial, but looking at > https://www.mercurial-scm.org/wiki/GitConcepts#Git.27s_staging_area > where the table says that index/staging area is not necessary in Mercurial > and suggests to use shelve instead that corresponds to Git's stash, > it would be possible to do the same with Hg as we first tried to do > with Git using stash: put changes to stash/shelve, commit and restore > the previous state. I suppose it might work, but then what happens when stashed/shelved changes start to conflict with the ones just committed? The stash can pop up with conflict markers. Maybe that's okay, I don't know. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 12:03 ` Dmitry Gutov @ 2022-09-01 20:13 ` Dmitry Gutov 0 siblings, 0 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-09-01 20:13 UTC (permalink / raw) To: Juri Linkov; +Cc: Eli Zaretskii, Lars Ingebrigtsen, emacs-devel On 30.08.2022 15:03, Dmitry Gutov wrote: > On 30.08.2022 10:20, Juri Linkov wrote: >> I am unfamiliar with Mercurial, but looking at >> https://www.mercurial-scm.org/wiki/GitConcepts#Git.27s_staging_area >> where the table says that index/staging area is not necessary in >> Mercurial >> and suggests to use shelve instead that corresponds to Git's stash, >> it would be possible to do the same with Hg as we first tried to do >> with Git using stash: put changes to stash/shelve, commit and restore >> the previous state. > > I suppose it might work, but then what happens when stashed/shelved > changes start to conflict with the ones just committed? The stash can > pop up with conflict markers. > > Maybe that's okay, I don't know. Oh BTW 'hg import -m <message> --bypass <filename>' might be the command we were looking for. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-29 16:46 ` Eli Zaretskii 2022-08-29 17:05 ` Dmitry Gutov @ 2022-08-30 13:25 ` Alfred M. Szmidt 2022-08-30 13:42 ` Po Lu ` (2 more replies) 1 sibling, 3 replies; 95+ messages in thread From: Alfred M. Szmidt @ 2022-08-30 13:25 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, juri, emacs-devel > > 'C-x v v' on a diff buffer commits it as a patch (bug#52349) > > Excellent! This is going to save me a lot of work in the future. Bother: This is only supported for Git, which is against the spirit of VC, and definitely against the spirit of "C-x v v". This is a pitty indeed. Specially seeing that it is easy enough to get this working for all VCSs. Here is a hack I wrote ages ago, and this works for just about everything, it doesn't do exactly what is needed (e.g, this works on a single file basis) but it isn't too hard to get that working. ===File ~/diff-commit-hunk.el=============================== ;;;; diff-commit-hunk.el --- commit parts of a hunk in `diff-mode' (require 'diff-mode) (defun current-buffer-file-name () (buffer-file-name (current-buffer))) (defun restore-source-file () (with-current-buffer (current-buffer) (erase-buffer) (insert-buffer "*diff-commit-hunk*") (write-file (current-buffer-file-name))) (remove-hook 'vc-checkin-hook 'restore-source-file)) (defmacro with-original-file (&rest body) "Reverts associated source file temporarily in a `diff-mode' buffer to the latest found in VCS, executes BODY and commits the changes back VCS." `(progn (save-excursion (diff-goto-source) (let ((buf (current-buffer))) (with-current-buffer (get-buffer-create "*diff-commit-hunk*") (erase-buffer) (insert-buffer buf))) (vc-revert-file (current-buffer-file-name))) ,@body (save-excursion (diff-goto-source) (write-file (current-buffer-file-name)) (add-hook 'vc-checkin-hook 'restore-source-file) (vc-checkin (list (current-buffer-file-name)) (vc-backend (current-buffer-file-name)))))) ;;;###autoload (defun diff-commit-hunk () "Revert associated source file to the latest version from VCS, and apply (and commit) current hunk." (interactive) (with-original-file (let ((diff-advance-after-apply-hunk nil)) (diff-apply-hunk)))) ;;;###autoload (defun diff-commit-all () "Like `diff-commit-hunk', but applies all hunks in the current diff buffer." (interactive) (with-original-file (goto-char (point-min)) (diff-hunk-next) ;Skip header. (while (not (eobp)) (diff-apply-hunk)))) (define-key diff-mode-map (kbd "s-a") #'diff-commit-hunk) (define-key diff-mode-map (kbd "H-a") #'diff-commit-all) ;;;; diff-commit-hunk.el ends here. ============================================================ ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 13:25 ` Alfred M. Szmidt @ 2022-08-30 13:42 ` Po Lu 2022-08-30 14:02 ` Alfred M. Szmidt 2022-10-25 19:22 ` Dmitry Gutov 2022-08-30 14:08 ` Dmitry Gutov 2022-09-04 7:26 ` Philip Kaludercic 2 siblings, 2 replies; 95+ messages in thread From: Po Lu @ 2022-08-30 13:42 UTC (permalink / raw) To: Alfred M. Szmidt; +Cc: Eli Zaretskii, larsi, juri, emacs-devel "Alfred M. Szmidt" <ams@gnu.org> writes: > This is a pitty indeed. Specially seeing that it is easy enough to > get this working for all VCSs. Here is a hack I wrote ages ago, and > this works for just about everything, it doesn't do exactly what is > needed (e.g, this works on a single file basis) but it isn't too hard > to get that working. If you have time, could you work on integrating that code with VC? I would, but currently I'm heavily occupied with some other code (which happens to use Subversion), and the feature would be quite convenient. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 13:42 ` Po Lu @ 2022-08-30 14:02 ` Alfred M. Szmidt 2022-10-25 19:22 ` Dmitry Gutov 1 sibling, 0 replies; 95+ messages in thread From: Alfred M. Szmidt @ 2022-08-30 14:02 UTC (permalink / raw) To: Po Lu; +Cc: eliz, larsi, juri, emacs-devel > This is a pitty indeed. Specially seeing that it is easy enough to > get this working for all VCSs. Here is a hack I wrote ages ago, and > this works for just about everything, it doesn't do exactly what is > needed (e.g, this works on a single file basis) but it isn't too hard > to get that working. If you have time, could you work on integrating that code with VC? I would, but currently I'm heavily occupied with some other code (which happens to use Subversion), and the feature would be quite convenient. I wish, I'm swamped at work :-( The feature as such is indispensible, and not having to care about the VC probobly even more so. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 13:42 ` Po Lu 2022-08-30 14:02 ` Alfred M. Szmidt @ 2022-10-25 19:22 ` Dmitry Gutov 1 sibling, 0 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-10-25 19:22 UTC (permalink / raw) To: Po Lu, Alfred M. Szmidt; +Cc: Eli Zaretskii, larsi, juri, emacs-devel On 30.08.2022 16:42, Po Lu wrote: > I would, but currently I'm heavily occupied with some other code (which > happens to use Subversion), and the feature would be quite convenient. I'm curious whether you've had the chance to test it with SVN by now. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 13:25 ` Alfred M. Szmidt 2022-08-30 13:42 ` Po Lu @ 2022-08-30 14:08 ` Dmitry Gutov 2022-08-30 14:20 ` Alfred M. Szmidt 2022-09-04 7:26 ` Philip Kaludercic 2 siblings, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 14:08 UTC (permalink / raw) To: Alfred M. Szmidt, Eli Zaretskii; +Cc: larsi, juri, emacs-devel On 30.08.2022 16:25, Alfred M. Szmidt wrote: > This is a pitty indeed. Specially seeing that it is easy enough to > get this working for all VCSs. Here is a hack I wrote ages ago, and > this works for just about everything, it doesn't do exactly what is > needed (e.g, this works on a single file basis) but it isn't too hard > to get that working. This indeed could work as an approach (I suggested /tmp, you are using a single buffer to back up contents, but the effect should be the same). Do you like the current behavior with Git, though? Or do you perhaps have the same misgivings as Eli? ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 14:08 ` Dmitry Gutov @ 2022-08-30 14:20 ` Alfred M. Szmidt 2022-08-30 14:38 ` Dmitry Gutov 0 siblings, 1 reply; 95+ messages in thread From: Alfred M. Szmidt @ 2022-08-30 14:20 UTC (permalink / raw) To: Dmitry Gutov; +Cc: eliz, larsi, juri, emacs-devel On 30.08.2022 16:25, Alfred M. Szmidt wrote: > This is a pitty indeed. Specially seeing that it is easy enough to > get this working for all VCSs. Here is a hack I wrote ages ago, and > this works for just about everything, it doesn't do exactly what is > needed (e.g, this works on a single file basis) but it isn't too hard > to get that working. This indeed could work as an approach (I suggested /tmp, you are using a single buffer to back up contents, but the effect should be the same). Yup, a temp file would make it work for multiple files. Do you like the current behavior with Git, though? Or do you perhaps have the same misgivings as Eli? I've not tried Juri's change, but from the point of maintaining vc-fossil it would mean that I need to figure out how to implement this vc-checkin-patch function (and likewise for anyone else), specially when there is no need to make this git specific for the most common case. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 14:20 ` Alfred M. Szmidt @ 2022-08-30 14:38 ` Dmitry Gutov 2022-08-30 15:16 ` Stefan Monnier ` (2 more replies) 0 siblings, 3 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 14:38 UTC (permalink / raw) To: Alfred M. Szmidt; +Cc: eliz, larsi, juri, emacs-devel On 30.08.2022 17:20, Alfred M. Szmidt wrote: > On 30.08.2022 16:25, Alfred M. Szmidt wrote: > > This is a pitty indeed. Specially seeing that it is easy enough to > > get this working for all VCSs. Here is a hack I wrote ages ago, and > > this works for just about everything, it doesn't do exactly what is > > needed (e.g, this works on a single file basis) but it isn't too hard > > to get that working. > > This indeed could work as an approach (I suggested /tmp, you are using a > single buffer to back up contents, but the effect should be the same). > > Yup, a temp file would make it work for multiple files. > > Do you like the current behavior with Git, though? Or do you perhaps > have the same misgivings as Eli? > > I've not tried Juri's change, but from the point of maintaining > vc-fossil it would mean that I need to figure out how to implement > this vc-checkin-patch function (and likewise for anyone else), > specially when there is no need to make this git specific for the most > common case. I can look into adapting this approach for other VCS if nobody beats me to it. As long as we fundamentally agree on the behavior. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 14:38 ` Dmitry Gutov @ 2022-08-30 15:16 ` Stefan Monnier 2022-08-30 15:26 ` Dmitry Gutov 2022-08-30 16:50 ` Eli Zaretskii 2022-08-30 16:37 ` Eli Zaretskii 2022-08-31 1:28 ` Po Lu 2 siblings, 2 replies; 95+ messages in thread From: Stefan Monnier @ 2022-08-30 15:16 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Alfred M. Szmidt, eliz, larsi, juri, emacs-devel > I can look into adapting this approach for other VCS if nobody beats me to > it. As long as we fundamentally agree on the behavior. Fine by me. I'll just mention that I think we should be careful to make sure no info is lost if Emacs crashes in the middle of the operation. Stefan ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 15:16 ` Stefan Monnier @ 2022-08-30 15:26 ` Dmitry Gutov 2022-08-30 15:33 ` Lars Ingebrigtsen 2022-08-30 18:36 ` Stefan Monnier 2022-08-30 16:50 ` Eli Zaretskii 1 sibling, 2 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 15:26 UTC (permalink / raw) To: Stefan Monnier; +Cc: Alfred M. Szmidt, eliz, larsi, juri, emacs-devel On 30.08.2022 18:16, Stefan Monnier wrote: >> I can look into adapting this approach for other VCS if nobody beats me to >> it. As long as we fundamentally agree on the behavior. > Fine by me. I'll just mention that I think we should be careful to make > sure no info is lost if Emacs crashes in the middle of the operation. File contents' backups should remain somewhere in /tmp, *shrug*. It might take some effort to find them, though. I think it's hard to implement this with even more safety somehow. Git makes it uniquely easier to do. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 15:26 ` Dmitry Gutov @ 2022-08-30 15:33 ` Lars Ingebrigtsen 2022-08-30 15:45 ` Dmitry Gutov 2022-08-30 18:36 ` Stefan Monnier 1 sibling, 1 reply; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-08-30 15:33 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, Alfred M. Szmidt, eliz, juri, emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > File contents' backups should remain somewhere in /tmp, *shrug*. What about stashing the files using the auto-save machinery? Then Emacs should be able to find them again if Emacs crashed during the operation. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 15:33 ` Lars Ingebrigtsen @ 2022-08-30 15:45 ` Dmitry Gutov 2022-08-30 15:53 ` Lars Ingebrigtsen 0 siblings, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 15:45 UTC (permalink / raw) To: Lars Ingebrigtsen Cc: Stefan Monnier, Alfred M. Szmidt, eliz, juri, emacs-devel On 30.08.2022 18:33, Lars Ingebrigtsen wrote: > Dmitry Gutov<dgutov@yandex.ru> writes: > >> File contents' backups should remain somewhere in /tmp,*shrug*. > What about stashing the files using the auto-save machinery? Then Emacs > should be able to find them again if Emacs crashed during the operation. The user might have backups disabled, though. That seems to require an additional fallback... ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 15:45 ` Dmitry Gutov @ 2022-08-30 15:53 ` Lars Ingebrigtsen 0 siblings, 0 replies; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-08-30 15:53 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Stefan Monnier, Alfred M. Szmidt, eliz, juri, emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > The user might have backups disabled, though. That seems to require an > additional fallback... We could force-enable them for this operation. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 15:26 ` Dmitry Gutov 2022-08-30 15:33 ` Lars Ingebrigtsen @ 2022-08-30 18:36 ` Stefan Monnier 2022-08-30 20:59 ` Dmitry Gutov 1 sibling, 1 reply; 95+ messages in thread From: Stefan Monnier @ 2022-08-30 18:36 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Alfred M. Szmidt, eliz, larsi, juri, emacs-devel Dmitry Gutov [2022-08-30 18:26:41] wrote: > On 30.08.2022 18:16, Stefan Monnier wrote: >>> I can look into adapting this approach for other VCS if nobody beats me to >>> it. As long as we fundamentally agree on the behavior. >> Fine by me. I'll just mention that I think we should be careful to make >> sure no info is lost if Emacs crashes in the middle of the operation. > File contents' backups should remain somewhere in /tmp, *shrug*. > It might take some effort to find them, though. We could do something like: run-the-vcs-diff-on-the FILES >VC-pending-changes patch -R <VC-pending-changes ...do the vc-checkin-patch... patch <VC-pending-changes rm VC-pending-changes or (closer to Git's behavior): tar -cf VC-pending-changes.tar FILES vc-revert FILES ...do the vc-checkin-patch... tar xf VC-pending-changes.tar rm VC-pending-changes.tar > I think it's hard to implement this with even more safety somehow. I'm just worried that the uncommmitted changes we need to temporarily remove are the rare parts that contain information that's not yet duplicated in the VCS's data, so if we only store it in an Emacs buffer it's vulnerable to an Emacs crash (which is something that *does* happen, especially for us poor souls who run with experimental code and with all assertions enabled :-). > Git makes it uniquely easier to do. Indeed. Stefan ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 18:36 ` Stefan Monnier @ 2022-08-30 20:59 ` Dmitry Gutov 0 siblings, 0 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 20:59 UTC (permalink / raw) To: Stefan Monnier; +Cc: Alfred M. Szmidt, eliz, larsi, juri, emacs-devel On 30.08.2022 21:36, Stefan Monnier wrote: > Dmitry Gutov [2022-08-30 18:26:41] wrote: >> On 30.08.2022 18:16, Stefan Monnier wrote: >>>> I can look into adapting this approach for other VCS if nobody beats me to >>>> it. As long as we fundamentally agree on the behavior. >>> Fine by me. I'll just mention that I think we should be careful to make >>> sure no info is lost if Emacs crashes in the middle of the operation. >> File contents' backups should remain somewhere in /tmp, *shrug*. >> It might take some effort to find them, though. > > We could do something like: > > run-the-vcs-diff-on-the FILES >VC-pending-changes > patch -R <VC-pending-changes > ...do the vc-checkin-patch... > patch <VC-pending-changes > rm VC-pending-changes > > or (closer to Git's behavior): > > tar -cf VC-pending-changes.tar FILES > vc-revert FILES > ...do the vc-checkin-patch... > tar xf VC-pending-changes.tar > rm VC-pending-changes.tar Except we probably can't rely on 'patch' or 'tar' being installed on the user's system. >> I think it's hard to implement this with even more safety somehow. > > I'm just worried that the uncommmitted changes we need to temporarily > remove are the rare parts that contain information that's not yet > duplicated in the VCS's data, so if we only store it in an Emacs buffer > it's vulnerable to an Emacs crash (which is something that *does* > happen, especially for us poor souls who run with experimental code and > with all assertions enabled :-). The /tmp approach implied 'cp', not storing contents of files in background buffers. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 15:16 ` Stefan Monnier 2022-08-30 15:26 ` Dmitry Gutov @ 2022-08-30 16:50 ` Eli Zaretskii 2022-08-31 6:51 ` Alfred M. Szmidt 1 sibling, 1 reply; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 16:50 UTC (permalink / raw) To: Stefan Monnier; +Cc: dgutov, ams, larsi, juri, emacs-devel > From: Stefan Monnier <monnier@iro.umontreal.ca> > Cc: "Alfred M. Szmidt" <ams@gnu.org>, eliz@gnu.org, larsi@gnus.org, > juri@jurta.org, emacs-devel@gnu.org > Date: Tue, 30 Aug 2022 11:16:17 -0400 > > > I can look into adapting this approach for other VCS if nobody beats me to > > it. As long as we fundamentally agree on the behavior. > > Fine by me. I'll just mention that I think we should be careful to make > sure no info is lost if Emacs crashes in the middle of the operation. Emacs never crashes. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 16:50 ` Eli Zaretskii @ 2022-08-31 6:51 ` Alfred M. Szmidt 0 siblings, 0 replies; 95+ messages in thread From: Alfred M. Szmidt @ 2022-08-31 6:51 UTC (permalink / raw) To: Eli Zaretskii; +Cc: monnier, dgutov, larsi, juri, emacs-devel > From: Stefan Monnier <monnier@iro.umontreal.ca> > Cc: "Alfred M. Szmidt" <ams@gnu.org>, eliz@gnu.org, larsi@gnus.org, > juri@jurta.org, emacs-devel@gnu.org > Date: Tue, 30 Aug 2022 11:16:17 -0400 > > > I can look into adapting this approach for other VCS if nobody beats me to > > it. As long as we fundamentally agree on the behavior. > > Fine by me. I'll just mention that I think we should be careful to make > sure no info is lost if Emacs crashes in the middle of the operation. Emacs never crashes. :-) But why is it important that no info is lost during a crash? *vc-diff* buffers, and in general any non-file associated buffer is always lost in the case of a crash. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 14:38 ` Dmitry Gutov 2022-08-30 15:16 ` Stefan Monnier @ 2022-08-30 16:37 ` Eli Zaretskii 2022-08-30 16:45 ` Lars Ingebrigtsen 2022-08-30 17:10 ` Dmitry Gutov 2022-08-31 1:28 ` Po Lu 2 siblings, 2 replies; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 16:37 UTC (permalink / raw) To: Dmitry Gutov; +Cc: ams, larsi, juri, emacs-devel > Date: Tue, 30 Aug 2022 17:38:27 +0300 > Cc: eliz@gnu.org, larsi@gnus.org, juri@jurta.org, emacs-devel@gnu.org > From: Dmitry Gutov <dgutov@yandex.ru> > > I can look into adapting this approach for other VCS if nobody beats me > to it. Thanks. > As long as we fundamentally agree on the behavior. I think the "C-x v D" step should be part of the command's execution. It makes little sense to me to ask the user to prepare the diffs in advance, since conceptually the changes are already on disk. Starting from diffs makes sense when you want to apply them first, and only after that commit. Which is not the case here. I understand that typing "C-x v v" in a diffs buffer is used here as an indication that this particular variant of "committing" is requested, but that's a weak justification, IMO. It could even backfire: the user could be in the diffs by sheer luck (or lack thereof), and type "C-x v v" without any intention to select parts of the changes. I think a special numeric argument (like "C-u 0", perhaps?) is more appropriate to signal the interactive selection of changes. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 16:37 ` Eli Zaretskii @ 2022-08-30 16:45 ` Lars Ingebrigtsen 2022-08-30 17:02 ` Eli Zaretskii 2022-08-30 17:10 ` Dmitry Gutov 1 sibling, 1 reply; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-08-30 16:45 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Dmitry Gutov, ams, juri, emacs-devel Eli Zaretskii <eliz@gnu.org> writes: > I think the "C-x v D" step should be part of the command's execution. > It makes little sense to me to ask the user to prepare the diffs in > advance, since conceptually the changes are already on disk. It's just an easy way to decide which changes are part of this commit. > I think a special numeric argument (like "C-u 0", perhaps?) is more > appropriate to signal the interactive selection of changes. And then it asks you for every hunk whether to include it in the commit or not? Or did you have something else in mind? ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 16:45 ` Lars Ingebrigtsen @ 2022-08-30 17:02 ` Eli Zaretskii 2022-08-30 17:11 ` Lars Ingebrigtsen 0 siblings, 1 reply; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 17:02 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: dgutov, ams, juri, emacs-devel > From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: Dmitry Gutov <dgutov@yandex.ru>, ams@gnu.org, juri@jurta.org, > emacs-devel@gnu.org > Date: Tue, 30 Aug 2022 18:45:18 +0200 > > Eli Zaretskii <eliz@gnu.org> writes: > > > I think the "C-x v D" step should be part of the command's execution. > > It makes little sense to me to ask the user to prepare the diffs in > > advance, since conceptually the changes are already on disk. > > It's just an easy way to decide which changes are part of this commit. I didn't say it shouldn't be displayed, I said the user shouldn't be required to prepare this display manually; the command should do it instead. > > I think a special numeric argument (like "C-u 0", perhaps?) is more > > appropriate to signal the interactive selection of changes. > > And then it asks you for every hunk whether to include it in the commit > or not? And then show a message to the user saying something like Edit the diffs to leave only the hunks you want to commit, then type C-c to go ahead and commit them. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 17:02 ` Eli Zaretskii @ 2022-08-30 17:11 ` Lars Ingebrigtsen 2022-08-30 17:34 ` Eli Zaretskii 2022-08-30 19:23 ` Sean Whitton 0 siblings, 2 replies; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-08-30 17:11 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dgutov, ams, juri, emacs-devel Eli Zaretskii <eliz@gnu.org> writes: > And then show a message to the user saying something like > > Edit the diffs to leave only the hunks you want to commit, > then type C-c to go ahead and commit them. In a recursive edit? Sure, that's a possible interface, but I don't really see much practical difference, except being less flexible. You're proposing a "start-a-selective-commit" command followed by `C-c C-c'. But today you can say `C-x v D' to make the diff buffer, but you can also make it in other ways, like `v' in the vc-dir buffer, or your own self-composed command to get the current tree's diff (I've got one on `H-d' that does some other stuff first and then presents me with the diff buffer). And then you an edit the buffer, and `C-x v v' to commit just the selected bits. It's more flexible, and not more work for the user than what you're proposing, as far as I can tell. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 17:11 ` Lars Ingebrigtsen @ 2022-08-30 17:34 ` Eli Zaretskii 2022-08-30 17:38 ` Lars Ingebrigtsen 2022-08-30 19:23 ` Sean Whitton 1 sibling, 1 reply; 95+ messages in thread From: Eli Zaretskii @ 2022-08-30 17:34 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: dgutov, ams, juri, emacs-devel > From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: dgutov@yandex.ru, ams@gnu.org, juri@jurta.org, emacs-devel@gnu.org > Date: Tue, 30 Aug 2022 19:11:27 +0200 > > Eli Zaretskii <eliz@gnu.org> writes: > > > And then show a message to the user saying something like > > > > Edit the diffs to leave only the hunks you want to commit, > > then type C-c to go ahead and commit them. > > In a recursive edit? Yes. > Sure, that's a possible interface, but I don't really see much practical > difference, except being less flexible. It's a more natural UI, IMO. > It's more flexible, and not more work for the user than what you're > proposing, as far as I can tell. But it's backward from the UI POV. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 17:34 ` Eli Zaretskii @ 2022-08-30 17:38 ` Lars Ingebrigtsen 0 siblings, 0 replies; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-08-30 17:38 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dgutov, ams, juri, emacs-devel Eli Zaretskii <eliz@gnu.org> writes: >> It's more flexible, and not more work for the user than what you're >> proposing, as far as I can tell. > > But it's backward from the UI POV. We could implement your proposed "start a selective commit of the root of the tree" command on top of what we have now pretty trivially. But what's implemented now is what I want to have. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 17:11 ` Lars Ingebrigtsen 2022-08-30 17:34 ` Eli Zaretskii @ 2022-08-30 19:23 ` Sean Whitton 1 sibling, 0 replies; 95+ messages in thread From: Sean Whitton @ 2022-08-30 19:23 UTC (permalink / raw) To: Lars Ingebrigtsen, Eli Zaretskii, dgutov, ams, juri, emacs-devel Hello, On Tue 30 Aug 2022 at 07:11PM +02, Lars Ingebrigtsen wrote: > Eli Zaretskii <eliz@gnu.org> writes: > >> And then show a message to the user saying something like >> >> Edit the diffs to leave only the hunks you want to commit, >> then type C-c to go ahead and commit them. > > In a recursive edit? > > Sure, that's a possible interface, but I don't really see much practical > difference, except being less flexible. You're proposing a > "start-a-selective-commit" command followed by `C-c C-c'. > > But today you can say `C-x v D' to make the diff buffer, but you can > also make it in other ways, like `v' in the vc-dir buffer, or your own > self-composed command to get the current tree's diff (I've got one on > `H-d' that does some other stuff first and then presents me with the > diff buffer). And then you an edit the buffer, and `C-x v v' to commit > just the selected bits. > > It's more flexible, and not more work for the user than what you're > proposing, as far as I can tell. I agree, and prefer how with the C-x v D approach you aren't in some incomplete or transient state, waiting for you to trim the diff. You can just do it when and if you want to. -- Sean Whitton ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 16:37 ` Eli Zaretskii 2022-08-30 16:45 ` Lars Ingebrigtsen @ 2022-08-30 17:10 ` Dmitry Gutov 1 sibling, 0 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-08-30 17:10 UTC (permalink / raw) To: Eli Zaretskii; +Cc: ams, larsi, juri, emacs-devel On 30.08.2022 19:37, Eli Zaretskii wrote: >> As long as we fundamentally agree on the behavior. > I think the "C-x v D" step should be part of the command's execution. > It makes little sense to me to ask the user to prepare the diffs in > advance, since conceptually the changes are already on disk. Starting > from diffs makes sense when you want to apply them first, and only > after that commit. Which is not the case here. The user can choose the files first (e.g. choose a directory from VC-Dir), or just pick one file they are currently editing. Then see the diff, and make the choice of making a commit from it. What you're suggesting could possibly improve discoverability (if the user manages to read the doc and find out about the meaning of C-0 prefix), but WRT flexibility, the effect can be negative. That is, unless we keep the new meaning of 'C-x v v' in diff buffers, the ability to commit an arbitrary diff (not applied yet) will go away. And it's what the author of the top message in this tree was particularly happy about. And as for the length of key sequences, in VC-Dir, 'D C-x v v' is about the same as 'C-0 C-x v v'. > I understand that typing "C-x v v" in a diffs buffer is used here as > an indication that this particular variant of "committing" is > requested, but that's a weak justification, IMO. It could even > backfire: the user could be in the diffs by sheer luck (or lack > thereof), and type "C-x v v" without any intention to select parts of > the changes. If they have no intention of creating a commit from that diff and press 'C-x v v' by accident, they can simply 'C-c C-k' out of the resulting Log-Edit buffer. That will bring them back to the diff, restoring the previous window configuration. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 14:38 ` Dmitry Gutov 2022-08-30 15:16 ` Stefan Monnier 2022-08-30 16:37 ` Eli Zaretskii @ 2022-08-31 1:28 ` Po Lu 2022-08-31 2:32 ` Dmitry Gutov 2 siblings, 1 reply; 95+ messages in thread From: Po Lu @ 2022-08-31 1:28 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Alfred M. Szmidt, eliz, larsi, juri, emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > I can look into adapting this approach for other VCS if nobody beats > me to it. As long as we fundamentally agree on the behavior. Thank you. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 1:28 ` Po Lu @ 2022-08-31 2:32 ` Dmitry Gutov 2022-08-31 6:53 ` Juri Linkov 2022-08-31 11:23 ` Eli Zaretskii 0 siblings, 2 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-08-31 2:32 UTC (permalink / raw) To: Po Lu; +Cc: Alfred M. Szmidt, eliz, larsi, juri, emacs-devel [-- Attachment #1: Type: text/plain, Size: 420 bytes --] On 31.08.2022 04:28, Po Lu wrote: > Dmitry Gutov<dgutov@yandex.ru> writes: > >> I can look into adapting this approach for other VCS if nobody beats >> me to it. As long as we fundamentally agree on the behavior. > Thank you. This seems to work. Might use some extra polish, though, if people also encounter the one weird error about "buffer already killed" that I saw a few times but then stopped after a restart. [-- Attachment #2: vc-default-checkin-patch.diff --] [-- Type: text/x-patch, Size: 1704 bytes --] diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el index d93be951a3..d6eda2b688 100644 --- a/lisp/vc/vc.el +++ b/lisp/vc/vc.el @@ -1667,6 +1667,41 @@ vc-checkin backend patch-string)) +(declare-function diff-apply-hunk "diff-mode" (&optional reverse)) + +(defun vc-default-checkin-patch (backend patch-string comment) + (with-current-buffer (get-buffer-create " *vc-checkin-patch*") + (erase-buffer) + (insert patch-string) + (diff-mode) + (pcase-let ((`(,backend ,files) (diff-vc-deduce-fileset)) + (root (vc-call-backend backend 'root default-directory)) + (tmpdir (make-temp-file "vc-checkin-patch" t))) + (dolist (f files) + (copy-file (expand-file-name f root) + (expand-file-name f tmpdir))) + (unwind-protect + (let ((default-directory root)) + (dolist (f files) + (with-current-buffer (get-file-buffer f) + (vc-revert-file f) + (revert-buffer t t t))) + (goto-char (point-min)) + (while (not (eobp)) + (diff-apply-hunk)) + (dolist (f files) + (with-current-buffer (get-file-buffer f) + (save-buffer 0))) + (vc-checkin files backend comment)) + (dolist (f files) + (copy-file (expand-file-name f tmpdir) + (expand-file-name f root) + t) + (with-current-buffer (get-file-buffer f) + (revert-buffer t t t)) + (delete-directory tmpdir t)) + (bury-buffer))))) + ;;; Additional entry points for examining version histories ;; (defun vc-default-diff-tree (backend dir rev1 rev2) ^ permalink raw reply related [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 2:32 ` Dmitry Gutov @ 2022-08-31 6:53 ` Juri Linkov 2022-08-31 9:55 ` Lars Ingebrigtsen 2022-08-31 11:11 ` Dmitry Gutov 2022-08-31 11:23 ` Eli Zaretskii 1 sibling, 2 replies; 95+ messages in thread From: Juri Linkov @ 2022-08-31 6:53 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Po Lu, Alfred M. Szmidt, eliz, larsi, emacs-devel >>> I can look into adapting this approach for other VCS if nobody beats >>> me to it. As long as we fundamentally agree on the behavior. >> Thank you. > > This seems to work. > > Might use some extra polish, though, if people also encounter the one weird > error about "buffer already killed" that I saw a few times but then stopped > after a restart. Thanks, looks universal enough to handle all backends. I've tried it out, but it gets stuck asking the same question: Hunk has already been applied; undo it? (y or n) because of the infinite loop in: (while (not (eobp)) (diff-apply-hunk)) After removing the loop, everything works fine. > +(defun vc-default-checkin-patch (backend patch-string comment) > + (with-current-buffer (get-buffer-create " *vc-checkin-patch*") > + (erase-buffer) > + (insert patch-string) > + (diff-mode) > + (pcase-let ((`(,backend ,files) (diff-vc-deduce-fileset)) > + (root (vc-call-backend backend 'root default-directory)) > + (tmpdir (make-temp-file "vc-checkin-patch" t))) I'm worried that after a crash, reboot might wipe out the /tmp dir. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 6:53 ` Juri Linkov @ 2022-08-31 9:55 ` Lars Ingebrigtsen 2022-08-31 10:35 ` Dmitry Gutov 2022-08-31 11:11 ` Dmitry Gutov 1 sibling, 1 reply; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-08-31 9:55 UTC (permalink / raw) To: Juri Linkov; +Cc: Dmitry Gutov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel Juri Linkov <juri@linkov.net> writes: >> Might use some extra polish, though, if people also encounter the one weird >> error about "buffer already killed" that I saw a few times but then stopped >> after a restart. > > Thanks, looks universal enough to handle all backends. I hope that the already-implemented safe version for Git doesn't get lost, though. Emacs' number one job is to never lose people's data, and the current code is safe in that regard. This new, more general code, does not look safe: If something goes wrong (like the computer going down) in the middle of this operation, we lose people's files, and that's not acceptable. And that's not unlikely, because a checkin in some VCs means talking over the network, so anything can fail (or hang), basically. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 9:55 ` Lars Ingebrigtsen @ 2022-08-31 10:35 ` Dmitry Gutov 2022-08-31 10:37 ` Lars Ingebrigtsen 0 siblings, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-31 10:35 UTC (permalink / raw) To: Lars Ingebrigtsen, Juri Linkov; +Cc: Po Lu, Alfred M. Szmidt, eliz, emacs-devel On 31.08.2022 12:55, Lars Ingebrigtsen wrote: > Juri Linkov <juri@linkov.net> writes: > >>> Might use some extra polish, though, if people also encounter the one weird >>> error about "buffer already killed" that I saw a few times but then stopped >>> after a restart. >> >> Thanks, looks universal enough to handle all backends. > > I hope that the already-implemented safe version for Git doesn't get > lost, though. Definitely not. It's simpler, faster and, by design, more reliable. > Emacs' number one job is to never lose people's data, and the current > code is safe in that regard. This new, more general code, does not look > safe: If something goes wrong (like the computer going down) in the > middle of this operation, we lose people's files, and that's not > acceptable. Those files have been copied to a directory, so they should always be recoverable, no? > And that's not unlikely, because a checkin in some VCs means talking > over the network, so anything can fail (or hang), basically. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 10:35 ` Dmitry Gutov @ 2022-08-31 10:37 ` Lars Ingebrigtsen 2022-08-31 11:09 ` Dmitry Gutov 0 siblings, 1 reply; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-08-31 10:37 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > > I hope that the already-implemented safe version for Git doesn't get > > lost, though. > > Definitely not. It's simpler, faster and, by design, more reliable. *phew* > Those files have been copied to a directory, so they should always be > recoverable, no? The /tmp file is routinely wiped, so the files should be copied to somewhere under ~/.emacs.d/ instead, or in the current directory, if possible. And there should be a mechanism to let you know that you can recover the files if something went wrong. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 10:37 ` Lars Ingebrigtsen @ 2022-08-31 11:09 ` Dmitry Gutov 2022-08-31 15:42 ` Lars Ingebrigtsen 0 siblings, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-31 11:09 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel On 31.08.2022 13:37, Lars Ingebrigtsen wrote: >> Those files have been copied to a directory, so they should always be >> recoverable, no? > The /tmp file is routinely wiped, so the files should be copied to > somewhere under ~/.emacs.d/ instead, or in the current directory, if > possible. And there should be a mechanism to let you know that you can > recover the files if something went wrong. A "hidden" directory under the project root is one option. Reusing the backups mechanism should also work but seemed like more of a pain to implement. But improvements welcome, of course. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 11:09 ` Dmitry Gutov @ 2022-08-31 15:42 ` Lars Ingebrigtsen 2022-08-31 16:24 ` Dmitry Gutov 0 siblings, 1 reply; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-08-31 15:42 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > A "hidden" directory under the project root is one option. It occurs to me that if we (i.e., you 😀) do this, you've almost implemented a general mechanism for stashing, so it'd be great if that was implemented fully. I.e., stashing the files somewhere handy, and give the stash a name, and there you are. (In this case the name would be automatically generated, of course.) ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 15:42 ` Lars Ingebrigtsen @ 2022-08-31 16:24 ` Dmitry Gutov 2022-08-31 16:39 ` Lars Ingebrigtsen 0 siblings, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-31 16:24 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel On 31.08.2022 18:42, Lars Ingebrigtsen wrote: > Dmitry Gutov<dgutov@yandex.ru> writes: > >> A "hidden" directory under the project root is one option. > It occurs to me that if we (i.e., you 😀) do this, you've almost I prefer "we" in this case. ;-) > implemented a general mechanism for stashing, so it'd be great if that > was implemented fully. I.e., stashing the files somewhere handy, and > give the stash a name, and there you are. (In this case the name would > be automatically generated, of course.) I don't use stashing commands from VC myself, but the stash mechanism is usually more complex, e.g. it usually knows how to merge changes (when the user has edited the file in some place and then pops the stash) and indicate conflicts. That's why a stash is usually represented as a diff, I guess. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 16:24 ` Dmitry Gutov @ 2022-08-31 16:39 ` Lars Ingebrigtsen 2022-08-31 18:02 ` Dmitry Gutov 0 siblings, 1 reply; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-08-31 16:39 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > I don't use stashing commands from VC myself, but the stash mechanism > is usually more complex, e.g. it usually knows how to merge changes > (when the user has edited the file in some place and then pops the > stash) and indicate conflicts. > > That's why a stash is usually represented as a diff, I guess. Yes. But I think you could just do that here, too? I.e., instead of copying and managing files, you can just write the `C-x v D' diff to a file, apply reverted diff, apply user-edited patch, commit, reverse edited patch, apply diff from file. It seems easier, even -- you don't have to manage a bunch of files. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 16:39 ` Lars Ingebrigtsen @ 2022-08-31 18:02 ` Dmitry Gutov 2022-09-01 10:16 ` Lars Ingebrigtsen 0 siblings, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-31 18:02 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel On 31.08.2022 19:39, Lars Ingebrigtsen wrote: > Dmitry Gutov <dgutov@yandex.ru> writes: > >> I don't use stashing commands from VC myself, but the stash mechanism >> is usually more complex, e.g. it usually knows how to merge changes >> (when the user has edited the file in some place and then pops the >> stash) and indicate conflicts. >> >> That's why a stash is usually represented as a diff, I guess. > > Yes. But I think you could just do that here, too? I.e., instead of > copying and managing files, you can just write the `C-x v D' diff to a > file, apply reverted diff, apply user-edited patch, commit, reverse > edited patch, apply diff from file. If you like, I can take this route as well. It seems a bit more error-prone, though: some unknown bug in diff-apply-hunk might make it fail at the end, or even corrupt the file contents silently. 'cp' seems more bullet-proof in that regard. > It seems easier, even -- you don't have to manage a bunch of files. True. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 18:02 ` Dmitry Gutov @ 2022-09-01 10:16 ` Lars Ingebrigtsen 2022-09-01 11:31 ` Dmitry Gutov 0 siblings, 1 reply; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-09-01 10:16 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: >> Yes. But I think you could just do that here, too? I.e., instead >> of >> copying and managing files, you can just write the `C-x v D' diff to a >> file, apply reverted diff, apply user-edited patch, commit, reverse >> edited patch, apply diff from file. > > If you like, I can take this route as well. It seems a bit more > error-prone, though: some unknown bug in diff-apply-hunk might make it > fail at the end, or even corrupt the file contents silently. > > 'cp' seems more bullet-proof in that regard. You should be able to get back to the previous state reliably by reverting to the repository state and then applying the saved patch, though. Which is basically what git does with stashes, anyway... ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-01 10:16 ` Lars Ingebrigtsen @ 2022-09-01 11:31 ` Dmitry Gutov 2022-09-01 11:45 ` Lars Ingebrigtsen 0 siblings, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-09-01 11:31 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel On 01.09.2022 13:16, Lars Ingebrigtsen wrote: > You should be able to get back to the previous state reliably by > reverting to the repository state and then applying the saved patch, > though. The step of "applying the saved patch" relies on our Lisp code being correct and being able to handle any diff. It's not an outrageous assumption, but an assumption nevertheless. > Which is basically what git does with stashes, anyway... Yup. Git is vastly better tested, though. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-01 11:31 ` Dmitry Gutov @ 2022-09-01 11:45 ` Lars Ingebrigtsen 2022-09-01 11:48 ` Dmitry Gutov 2022-09-01 13:26 ` Robert Pluim 0 siblings, 2 replies; 95+ messages in thread From: Lars Ingebrigtsen @ 2022-09-01 11:45 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > The step of "applying the saved patch" relies on our Lisp code being > correct and being able to handle any diff. It's not an outrageous > assumption, but an assumption nevertheless. Oh, I was assuming that we were assuming that we had "patch" installed for this to work. I don't think that's unreasonable for new functionality like this. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-01 11:45 ` Lars Ingebrigtsen @ 2022-09-01 11:48 ` Dmitry Gutov 2022-09-01 11:57 ` Eli Zaretskii 2022-10-17 0:11 ` Dmitry Gutov 2022-09-01 13:26 ` Robert Pluim 1 sibling, 2 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-09-01 11:48 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel On 01.09.2022 14:45, Lars Ingebrigtsen wrote: > Dmitry Gutov<dgutov@yandex.ru> writes: > >> The step of "applying the saved patch" relies on our Lisp code being >> correct and being able to handle any diff. It's not an outrageous >> assumption, but an assumption nevertheless. > Oh, I was assuming that we were assuming that we had "patch" installed > for this to work. I don't think that's unreasonable for new > functionality like this. I suggested this upthread but didn't get any affirmative response. This would be easier, yes. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-01 11:48 ` Dmitry Gutov @ 2022-09-01 11:57 ` Eli Zaretskii 2022-10-17 0:11 ` Dmitry Gutov 1 sibling, 0 replies; 95+ messages in thread From: Eli Zaretskii @ 2022-09-01 11:57 UTC (permalink / raw) To: Dmitry Gutov; +Cc: larsi, juri, luangruo, ams, emacs-devel > Date: Thu, 1 Sep 2022 14:48:32 +0300 > Cc: Juri Linkov <juri@linkov.net>, Po Lu <luangruo@yahoo.com>, > "Alfred M. Szmidt" <ams@gnu.org>, eliz@gnu.org, emacs-devel@gnu.org > From: Dmitry Gutov <dgutov@yandex.ru> > > On 01.09.2022 14:45, Lars Ingebrigtsen wrote: > > Oh, I was assuming that we were assuming that we had "patch" installed > > for this to work. I don't think that's unreasonable for new > > functionality like this. > > I suggested this upthread but didn't get any affirmative response. > > This would be easier, yes. If it will be easier, by all means go for it. I, too, don't think it's unreasonable. Even on MS-Windows, there's a port of GNU Patch, and people who have Git for Windows installed get Patch as part of the deal. Thanks. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-01 11:48 ` Dmitry Gutov 2022-09-01 11:57 ` Eli Zaretskii @ 2022-10-17 0:11 ` Dmitry Gutov 2022-10-17 19:17 ` Juri Linkov 1 sibling, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-10-17 0:11 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel On 01.09.2022 14:48, Dmitry Gutov wrote: > On 01.09.2022 14:45, Lars Ingebrigtsen wrote: >> Dmitry Gutov<dgutov@yandex.ru> writes: >> >>> The step of "applying the saved patch" relies on our Lisp code being >>> correct and being able to handle any diff. It's not an outrageous >>> assumption, but an assumption nevertheless. >> Oh, I was assuming that we were assuming that we had "patch" installed >> for this to work. I don't think that's unreasonable for new >> functionality like this. > > I suggested this upthread but didn't get any affirmative response. > > This would be easier, yes. So... I ended up using 'patch', but not for backing up existing uncommitted changes. It's the same reason as why 'hg shelve' and 'git stash' are a poor fit: when we try to apply the patch at the end, we can get a conflict (and not every such conflict will be easy enough to fix by hand). The file-copying approach matches Git's implementation well, and hopefully will work with various backends, including CVS. I only tested with Hg for now. Feedback welcome. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-10-17 0:11 ` Dmitry Gutov @ 2022-10-17 19:17 ` Juri Linkov 0 siblings, 0 replies; 95+ messages in thread From: Juri Linkov @ 2022-10-17 19:17 UTC (permalink / raw) To: Dmitry Gutov Cc: Lars Ingebrigtsen, Po Lu, Alfred M. Szmidt, eliz, emacs-devel > The file-copying approach matches Git's implementation well, and hopefully > will work with various backends, including CVS. I only tested with Hg for > now. > > Feedback welcome. Thanks, confirmed to work with bzr. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-01 11:45 ` Lars Ingebrigtsen 2022-09-01 11:48 ` Dmitry Gutov @ 2022-09-01 13:26 ` Robert Pluim 2022-09-01 13:28 ` Dmitry Gutov 2022-09-03 2:51 ` Richard Stallman 1 sibling, 2 replies; 95+ messages in thread From: Robert Pluim @ 2022-09-01 13:26 UTC (permalink / raw) To: Lars Ingebrigtsen Cc: Dmitry Gutov, Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel >>>>> On Thu, 01 Sep 2022 13:45:57 +0200, Lars Ingebrigtsen <larsi@gnus.org> said: Lars> Dmitry Gutov <dgutov@yandex.ru> writes: >> The step of "applying the saved patch" relies on our Lisp code being >> correct and being able to handle any diff. It's not an outrageous >> assumption, but an assumption nevertheless. Lars> Oh, I was assuming that we were assuming that we had "patch" installed Lars> for this to work. I don't think that's unreasonable for new Lars> functionality like this. 'git apply' will work as a replacement for 'patch' as well. Robert -- ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-01 13:26 ` Robert Pluim @ 2022-09-01 13:28 ` Dmitry Gutov 2022-09-01 13:32 ` Robert Pluim 2022-09-03 2:51 ` Richard Stallman 1 sibling, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-09-01 13:28 UTC (permalink / raw) To: Robert Pluim, Lars Ingebrigtsen Cc: Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel On 01.09.2022 16:26, Robert Pluim wrote: > 'git apply' will work as a replacement for 'patch' as well. We're talking about a fallback implementation that will work across backends. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-01 13:28 ` Dmitry Gutov @ 2022-09-01 13:32 ` Robert Pluim 0 siblings, 0 replies; 95+ messages in thread From: Robert Pluim @ 2022-09-01 13:32 UTC (permalink / raw) To: Dmitry Gutov Cc: Lars Ingebrigtsen, Juri Linkov, Po Lu, Alfred M. Szmidt, eliz, emacs-devel >>>>> On Thu, 1 Sep 2022 16:28:09 +0300, Dmitry Gutov <dgutov@yandex.ru> said: Dmitry> On 01.09.2022 16:26, Robert Pluim wrote: >> 'git apply' will work as a replacement for 'patch' as well. Dmitry> We're talking about a fallback implementation that will work across Dmitry> backends. I was too succinct. If the implementation at some point calls for "patch"-like functionality, but "patch" is not available but "git apply" is, then we could fall back to "git apply" (and the equivalent commands for other backends if they exist). Robert -- ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-01 13:26 ` Robert Pluim 2022-09-01 13:28 ` Dmitry Gutov @ 2022-09-03 2:51 ` Richard Stallman 2022-09-04 12:27 ` Alfred M. Szmidt 1 sibling, 1 reply; 95+ messages in thread From: Richard Stallman @ 2022-09-03 2:51 UTC (permalink / raw) To: Robert Pluim; +Cc: larsi, dgutov, juri, luangruo, ams, eliz, emacs-devel [[[ To any NSA and FBI agents reading my email: please consider ]]] [[[ whether defending the US Constitution against all enemies, ]]] [[[ foreign or domestic, requires you to follow Snowden's example. ]]] > 'git apply' will work as a replacement for 'patch' as well. Will it work with all the back ends that VC supports? If it only works with git, it is not a very useful fallback. Note that it would add the requirement for git to be installed. -- Dr Richard Stallman (https://stallman.org) Chief GNUisance of the GNU Project (https://gnu.org) Founder, Free Software Foundation (https://fsf.org) Internet Hall-of-Famer (https://internethalloffame.org) ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-03 2:51 ` Richard Stallman @ 2022-09-04 12:27 ` Alfred M. Szmidt 0 siblings, 0 replies; 95+ messages in thread From: Alfred M. Szmidt @ 2022-09-04 12:27 UTC (permalink / raw) To: rms; +Cc: rpluim, larsi, dgutov, juri, luangruo, eliz, emacs-devel > 'git apply' will work as a replacement for 'patch' as well. Will it work with all the back ends that VC supports? If it only works with git, it is not a very useful fallback. It should; `git apply' doesn't require that directory and files it works on is a git tree for its most basic functionality. What might be an issue is if .git/config is used, and any configuration values are set that change the behaviour of `git apply'. GNU patch has nicer features than `git apply' too, and is a standard Unix program, it makes more sense to depend on patch than on `git apply'. Note that it would add the requirement for git to be installed. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 6:53 ` Juri Linkov 2022-08-31 9:55 ` Lars Ingebrigtsen @ 2022-08-31 11:11 ` Dmitry Gutov 2022-08-31 16:06 ` Juri Linkov 1 sibling, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-31 11:11 UTC (permalink / raw) To: Juri Linkov; +Cc: Po Lu, Alfred M. Szmidt, eliz, larsi, emacs-devel On 31.08.2022 09:53, Juri Linkov wrote: >>>> I can look into adapting this approach for other VCS if nobody beats >>>> me to it. As long as we fundamentally agree on the behavior. >>> Thank you. >> This seems to work. >> >> Might use some extra polish, though, if people also encounter the one weird >> error about "buffer already killed" that I saw a few times but then stopped >> after a restart. > Thanks, looks universal enough to handle all backends. > I've tried it out, but it gets stuck asking the same question: > > Hunk has already been applied; undo it? (y or n) > > because of the infinite loop in: > > (while (not (eobp)) > (diff-apply-hunk)) > > After removing the loop, everything works fine. Do you perhaps have customized diff-advance-after-apply-hunk to nil? I guess a let-binding is in order. But a loop is necessary because the diff can have more than one hunk, isn't it? >> +(defun vc-default-checkin-patch (backend patch-string comment) >> + (with-current-buffer (get-buffer-create "*vc-checkin-patch*") >> + (erase-buffer) >> + (insert patch-string) >> + (diff-mode) >> + (pcase-let ((`(,backend ,files) (diff-vc-deduce-fileset)) >> + (root (vc-call-backend backend 'root default-directory)) >> + (tmpdir (make-temp-file "vc-checkin-patch" t))) > I'm worried that after a crash, reboot might wipe out the /tmp dir. Creating the tmp directory inside root is also an option. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 11:11 ` Dmitry Gutov @ 2022-08-31 16:06 ` Juri Linkov 2022-08-31 16:21 ` Dmitry Gutov 0 siblings, 1 reply; 95+ messages in thread From: Juri Linkov @ 2022-08-31 16:06 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Po Lu, Alfred M. Szmidt, eliz, larsi, emacs-devel >> I've tried it out, but it gets stuck asking the same question: >> Hunk has already been applied; undo it? (y or n) >> because of the infinite loop in: >> (while (not (eobp)) >> (diff-apply-hunk)) >> After removing the loop, everything works fine. > > Do you perhaps have customized diff-advance-after-apply-hunk to nil? > I guess a let-binding is in order. diff-advance-after-apply-hunk is still t. > But a loop is necessary because the diff can have more than one hunk, isn't > it? A loop is necessary indeed. The problem is that I tried it with Bzr, but it has such a bug that `bzr diff` adds an extra line at the end, so it's not (eobp) after the last diff hunk. After removing the final newline in the *vc-diff* buffer the loop can be finished. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 16:06 ` Juri Linkov @ 2022-08-31 16:21 ` Dmitry Gutov 2022-08-31 16:36 ` Eli Zaretskii 2022-08-31 16:41 ` Dmitry Gutov 0 siblings, 2 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-08-31 16:21 UTC (permalink / raw) To: Juri Linkov; +Cc: Po Lu, Alfred M. Szmidt, eliz, larsi, emacs-devel On 31.08.2022 19:06, Juri Linkov wrote: >> But a loop is necessary because the diff can have more than one hunk, isn't >> it? > A loop is necessary indeed. The problem is that I tried it with Bzr, > but it has such a bug that `bzr diff` adds an extra line at the end, > so it's not (eobp) after the last diff hunk. After removing the > final newline in the*vc-diff* buffer the loop can be finished. Hmm, so how do we apply the full diff in a backend-agnostic fashion? Should we delegate to 'patch', then? And require it to be installed. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 16:21 ` Dmitry Gutov @ 2022-08-31 16:36 ` Eli Zaretskii 2022-10-16 23:53 ` Dmitry Gutov 2022-08-31 16:41 ` Dmitry Gutov 1 sibling, 1 reply; 95+ messages in thread From: Eli Zaretskii @ 2022-08-31 16:36 UTC (permalink / raw) To: Dmitry Gutov; +Cc: juri, luangruo, ams, larsi, emacs-devel > Date: Wed, 31 Aug 2022 19:21:12 +0300 > Cc: Po Lu <luangruo@yahoo.com>, "Alfred M. Szmidt" <ams@gnu.org>, > eliz@gnu.org, larsi@gnus.org, emacs-devel@gnu.org > From: Dmitry Gutov <dgutov@yandex.ru> > > On 31.08.2022 19:06, Juri Linkov wrote: > >> But a loop is necessary because the diff can have more than one hunk, isn't > >> it? > > A loop is necessary indeed. The problem is that I tried it with Bzr, > > but it has such a bug that `bzr diff` adds an extra line at the end, > > so it's not (eobp) after the last diff hunk. After removing the > > final newline in the*vc-diff* buffer the loop can be finished. > > Hmm, so how do we apply the full diff in a backend-agnostic fashion? > > Should we delegate to 'patch', then? And require it to be installed. Shouldn't the code which looks for hunks be resilient to such minor differences? Why does it fail if there's an extra line at the end? Does that line look anything like another hunk? ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 16:36 ` Eli Zaretskii @ 2022-10-16 23:53 ` Dmitry Gutov 2022-10-16 23:54 ` Dmitry Gutov 0 siblings, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-10-16 23:53 UTC (permalink / raw) To: Eli Zaretskii; +Cc: juri, luangruo, ams, larsi, emacs-devel On 31.08.2022 19:36, Eli Zaretskii wrote: >> Date: Wed, 31 Aug 2022 19:21:12 +0300 >> Cc: Po Lu<luangruo@yahoo.com>, "Alfred M. Szmidt"<ams@gnu.org>, >> eliz@gnu.org,larsi@gnus.org,emacs-devel@gnu.org >> From: Dmitry Gutov<dgutov@yandex.ru> >> >> On 31.08.2022 19:06, Juri Linkov wrote: >>>> But a loop is necessary because the diff can have more than one hunk, isn't >>>> it? >>> A loop is necessary indeed. The problem is that I tried it with Bzr, >>> but it has such a bug that `bzr diff` adds an extra line at the end, >>> so it's not (eobp) after the last diff hunk. After removing the >>> final newline in the*vc-diff* buffer the loop can be finished. >> Hmm, so how do we apply the full diff in a backend-agnostic fashion? >> >> Should we delegate to 'patch', then? And require it to be installed. > Shouldn't the code which looks for hunks be resilient to such minor > differences? Why does it fail if there's an extra line at the end? > Does that line look anything like another hunk? I'm not sure how that line is supposed to look (I haven't been able to reproduce it with the one Bzr repo I had), but it should be possible to use a loop like (while (or (bobp) (looking-at diff-hunk-header-re) (diff-apply-hunk)) Anyway, I went ahead with using 'patch' for now. At least it will show simpler errors if user tries to commit patch with broken syntax. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-10-16 23:53 ` Dmitry Gutov @ 2022-10-16 23:54 ` Dmitry Gutov 0 siblings, 0 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-10-16 23:54 UTC (permalink / raw) To: Eli Zaretskii; +Cc: juri, luangruo, ams, larsi, emacs-devel On 17.10.2022 02:53, Dmitry Gutov wrote: > I'm not sure how that line is supposed to look (I haven't been able to > reproduce it with the one Bzr repo I had) Ah, I see it now. It's an extra newline. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 16:21 ` Dmitry Gutov 2022-08-31 16:36 ` Eli Zaretskii @ 2022-08-31 16:41 ` Dmitry Gutov 1 sibling, 0 replies; 95+ messages in thread From: Dmitry Gutov @ 2022-08-31 16:41 UTC (permalink / raw) To: Juri Linkov; +Cc: Po Lu, Alfred M. Szmidt, eliz, larsi, emacs-devel On 31.08.2022 19:21, Dmitry Gutov wrote: > On 31.08.2022 19:06, Juri Linkov wrote: >>> But a loop is necessary because the diff can have more than one hunk, >>> isn't >>> it? >> A loop is necessary indeed. The problem is that I tried it with Bzr, >> but it has such a bug that `bzr diff` adds an extra line at the end, >> so it's not (eobp) after the last diff hunk. After removing the >> final newline in the*vc-diff* buffer the loop can be finished. > > Hmm, so how do we apply the full diff in a backend-agnostic fashion? > > Should we delegate to 'patch', then? And require it to be installed. I suppose we could check whether every diff-apply-hunk advances point, though. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 2:32 ` Dmitry Gutov 2022-08-31 6:53 ` Juri Linkov @ 2022-08-31 11:23 ` Eli Zaretskii 2022-08-31 11:57 ` Dmitry Gutov 1 sibling, 1 reply; 95+ messages in thread From: Eli Zaretskii @ 2022-08-31 11:23 UTC (permalink / raw) To: Dmitry Gutov; +Cc: luangruo, ams, larsi, juri, emacs-devel > Date: Wed, 31 Aug 2022 05:32:42 +0300 > Cc: "Alfred M. Szmidt" <ams@gnu.org>, eliz@gnu.org, larsi@gnus.org, > juri@jurta.org, emacs-devel@gnu.org > From: Dmitry Gutov <dgutov@yandex.ru> > > >> I can look into adapting this approach for other VCS if nobody beats > >> me to it. As long as we fundamentally agree on the behavior. > > Thank you. > > This seems to work. Thanks. > + (dolist (f files) > + (copy-file (expand-file-name f root) > + (expand-file-name f tmpdir))) What if f includes leading directories, as in "lisp/foo.el"? If tmpdir doesn't have a subdirectory 'lisp', the copy-file will fail, no? > + (unwind-protect > + (let ((default-directory root)) > + (dolist (f files) > + (with-current-buffer (get-file-buffer f) ^^^^^^^^^^^^^^^^^^^ This assumes that all the files are visited in buffers? Is that guaranteed? ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 11:23 ` Eli Zaretskii @ 2022-08-31 11:57 ` Dmitry Gutov 2022-08-31 12:36 ` Eli Zaretskii 0 siblings, 1 reply; 95+ messages in thread From: Dmitry Gutov @ 2022-08-31 11:57 UTC (permalink / raw) To: Eli Zaretskii; +Cc: luangruo, ams, larsi, juri, emacs-devel On 31.08.2022 14:23, Eli Zaretskii wrote: >> Date: Wed, 31 Aug 2022 05:32:42 +0300 >> Cc: "Alfred M. Szmidt" <ams@gnu.org>, eliz@gnu.org, larsi@gnus.org, >> juri@jurta.org, emacs-devel@gnu.org >> From: Dmitry Gutov <dgutov@yandex.ru> >> >>>> I can look into adapting this approach for other VCS if nobody beats >>>> me to it. As long as we fundamentally agree on the behavior. >>> Thank you. >> >> This seems to work. > > Thanks. > >> + (dolist (f files) >> + (copy-file (expand-file-name f root) >> + (expand-file-name f tmpdir))) > > What if f includes leading directories, as in "lisp/foo.el"? If > tmpdir doesn't have a subdirectory 'lisp', the copy-file will fail, > no? That's unfortunate. >> + (unwind-protect >> + (let ((default-directory root)) >> + (dolist (f files) >> + (with-current-buffer (get-file-buffer f) > ^^^^^^^^^^^^^^^^^^^ > This assumes that all the files are visited in buffers? Is that > guaranteed? Right. That might account for that one-weird-bug I've seen once. ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-31 11:57 ` Dmitry Gutov @ 2022-08-31 12:36 ` Eli Zaretskii 0 siblings, 0 replies; 95+ messages in thread From: Eli Zaretskii @ 2022-08-31 12:36 UTC (permalink / raw) To: Dmitry Gutov; +Cc: luangruo, ams, larsi, juri, emacs-devel > Date: Wed, 31 Aug 2022 14:57:38 +0300 > Cc: luangruo@yahoo.com, ams@gnu.org, larsi@gnus.org, juri@jurta.org, > emacs-devel@gnu.org > From: Dmitry Gutov <dgutov@yandex.ru> > > >> + (dolist (f files) > >> + (copy-file (expand-file-name f root) > >> + (expand-file-name f tmpdir))) > > > > What if f includes leading directories, as in "lisp/foo.el"? If > > tmpdir doesn't have a subdirectory 'lisp', the copy-file will fail, > > no? > > That's unfortunate. I think you need to precede the copy-file call by (make-directory (file-name-directory (expand-file-name f tmpdir)) t) ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-08-30 13:25 ` Alfred M. Szmidt 2022-08-30 13:42 ` Po Lu 2022-08-30 14:08 ` Dmitry Gutov @ 2022-09-04 7:26 ` Philip Kaludercic 2022-09-04 12:27 ` Alfred M. Szmidt 2 siblings, 1 reply; 95+ messages in thread From: Philip Kaludercic @ 2022-09-04 7:26 UTC (permalink / raw) To: Alfred M. Szmidt; +Cc: Eli Zaretskii, larsi, juri, emacs-devel "Alfred M. Szmidt" <ams@gnu.org> writes: > > > 'C-x v v' on a diff buffer commits it as a patch (bug#52349) > > > > Excellent! This is going to save me a lot of work in the future. > > Bother: This is only supported for Git, which is against the spirit of > VC, and definitely against the spirit of "C-x v v". > > This is a pitty indeed. Specially seeing that it is easy enough to > get this working for all VCSs. Here is a hack I wrote ages ago, and > this works for just about everything, it doesn't do exactly what is > needed (e.g, this works on a single file basis) but it isn't too hard > to get that working. This feature looks very interesting to me. Could it be added to vc or as an package to GNU ELPA? > ===File ~/diff-commit-hunk.el=============================== > ;;;; diff-commit-hunk.el --- commit parts of a hunk in `diff-mode' > > (require 'diff-mode) > > (defun current-buffer-file-name () > (buffer-file-name (current-buffer))) > > (defun restore-source-file () > (with-current-buffer (current-buffer) > (erase-buffer) > (insert-buffer "*diff-commit-hunk*") > (write-file (current-buffer-file-name))) > (remove-hook 'vc-checkin-hook 'restore-source-file)) > > (defmacro with-original-file (&rest body) > "Reverts associated source file temporarily in a `diff-mode' > buffer to the latest found in VCS, executes BODY and commits the > changes back VCS." > `(progn > (save-excursion > (diff-goto-source) > (let ((buf (current-buffer))) > (with-current-buffer (get-buffer-create "*diff-commit-hunk*") > (erase-buffer) > (insert-buffer buf))) > (vc-revert-file (current-buffer-file-name))) > ,@body > (save-excursion > (diff-goto-source) > (write-file (current-buffer-file-name)) > (add-hook 'vc-checkin-hook 'restore-source-file) > (vc-checkin (list (current-buffer-file-name)) > (vc-backend (current-buffer-file-name)))))) > > ;;;###autoload > (defun diff-commit-hunk () > "Revert associated source file to the latest version from VCS, > and apply (and commit) current hunk." > (interactive) > (with-original-file > (let ((diff-advance-after-apply-hunk nil)) > (diff-apply-hunk)))) > > ;;;###autoload > (defun diff-commit-all () > "Like `diff-commit-hunk', but applies all hunks in the current > diff buffer." > (interactive) > (with-original-file > (goto-char (point-min)) > (diff-hunk-next) ;Skip header. > (while (not (eobp)) > (diff-apply-hunk)))) > > (define-key diff-mode-map (kbd "s-a") #'diff-commit-hunk) > (define-key diff-mode-map (kbd "H-a") #'diff-commit-all) > > ;;;; diff-commit-hunk.el ends here. > ============================================================ ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-04 7:26 ` Philip Kaludercic @ 2022-09-04 12:27 ` Alfred M. Szmidt 2022-09-04 17:07 ` Philip Kaludercic 0 siblings, 1 reply; 95+ messages in thread From: Alfred M. Szmidt @ 2022-09-04 12:27 UTC (permalink / raw) To: Philip Kaludercic; +Cc: eliz, larsi, juri, emacs-devel > > > 'C-x v v' on a diff buffer commits it as a patch (bug#52349) > > > > Excellent! This is going to save me a lot of work in the future. > > Bother: This is only supported for Git, which is against the spirit of > VC, and definitely against the spirit of "C-x v v". > > This is a pitty indeed. Specially seeing that it is easy enough to > get this working for all VCSs. Here is a hack I wrote ages ago, and > this works for just about everything, it doesn't do exactly what is > needed (e.g, this works on a single file basis) but it isn't too hard > to get that working. This feature looks very interesting to me. Could it be added to vc or as an package to GNU ELPA? Isn't it the same feature as being implemented (in a better manner)? > ===File ~/diff-commit-hunk.el=============================== > ;;;; diff-commit-hunk.el --- commit parts of a hunk in `diff-mode' > > (require 'diff-mode) > > (defun current-buffer-file-name () > (buffer-file-name (current-buffer))) > > (defun restore-source-file () > (with-current-buffer (current-buffer) > (erase-buffer) > (insert-buffer "*diff-commit-hunk*") > (write-file (current-buffer-file-name))) > (remove-hook 'vc-checkin-hook 'restore-source-file)) > > (defmacro with-original-file (&rest body) > "Reverts associated source file temporarily in a `diff-mode' > buffer to the latest found in VCS, executes BODY and commits the > changes back VCS." > `(progn > (save-excursion > (diff-goto-source) > (let ((buf (current-buffer))) > (with-current-buffer (get-buffer-create "*diff-commit-hunk*") > (erase-buffer) > (insert-buffer buf))) > (vc-revert-file (current-buffer-file-name))) > ,@body > (save-excursion > (diff-goto-source) > (write-file (current-buffer-file-name)) > (add-hook 'vc-checkin-hook 'restore-source-file) > (vc-checkin (list (current-buffer-file-name)) > (vc-backend (current-buffer-file-name)))))) > > ;;;###autoload > (defun diff-commit-hunk () > "Revert associated source file to the latest version from VCS, > and apply (and commit) current hunk." > (interactive) > (with-original-file > (let ((diff-advance-after-apply-hunk nil)) > (diff-apply-hunk)))) > > ;;;###autoload > (defun diff-commit-all () > "Like `diff-commit-hunk', but applies all hunks in the current > diff buffer." > (interactive) > (with-original-file > (goto-char (point-min)) > (diff-hunk-next) ;Skip header. > (while (not (eobp)) > (diff-apply-hunk)))) > > (define-key diff-mode-map (kbd "s-a") #'diff-commit-hunk) > (define-key diff-mode-map (kbd "H-a") #'diff-commit-all) > > ;;;; diff-commit-hunk.el ends here. > ============================================================ ^ permalink raw reply [flat|nested] 95+ messages in thread
* Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) 2022-09-04 12:27 ` Alfred M. Szmidt @ 2022-09-04 17:07 ` Philip Kaludercic 0 siblings, 0 replies; 95+ messages in thread From: Philip Kaludercic @ 2022-09-04 17:07 UTC (permalink / raw) To: Alfred M. Szmidt; +Cc: eliz, larsi, juri, emacs-devel "Alfred M. Szmidt" <ams@gnu.org> writes: > > > > 'C-x v v' on a diff buffer commits it as a patch (bug#52349) > > > > > > Excellent! This is going to save me a lot of work in the future. > > > > Bother: This is only supported for Git, which is against the spirit of > > VC, and definitely against the spirit of "C-x v v". > > > > This is a pitty indeed. Specially seeing that it is easy enough to > > get this working for all VCSs. Here is a hack I wrote ages ago, and > > this works for just about everything, it doesn't do exactly what is > > needed (e.g, this works on a single file basis) but it isn't too hard > > to get that working. > > This feature looks very interesting to me. Could it be added to vc or > as an package to GNU ELPA? > > Isn't it the same feature as being implemented (in a better manner)? If it is then I missed the message, in which case my question should be ignored. ^ permalink raw reply [flat|nested] 95+ messages in thread
end of thread, other threads:[~2022-10-25 19:22 UTC | newest] Thread overview: 95+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <166171593185.16640.41619657947456727@vcs2.savannah.gnu.org> [not found] ` <20220828194533.23A6BC00889@vcs2.savannah.gnu.org> 2022-08-28 21:18 ` master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) Sean Whitton 2022-08-29 4:50 ` Manuel Uberti 2022-08-29 16:40 ` Daniel Martín 2022-08-29 14:48 ` Lars Ingebrigtsen 2022-08-29 16:46 ` Eli Zaretskii 2022-08-29 17:05 ` Dmitry Gutov 2022-08-29 17:41 ` Eli Zaretskii 2022-08-29 19:03 ` Tassilo Horn 2022-08-29 19:31 ` Eli Zaretskii 2022-08-29 20:36 ` Dmitry Gutov 2022-08-30 11:25 ` Eli Zaretskii 2022-08-30 12:35 ` Dmitry Gutov 2022-08-30 13:01 ` Eli Zaretskii 2022-08-30 13:52 ` Dmitry Gutov 2022-08-30 16:20 ` Eli Zaretskii 2022-08-30 17:33 ` Dmitry Gutov 2022-08-30 16:47 ` Juri Linkov 2022-08-30 17:05 ` Eli Zaretskii 2022-08-30 17:11 ` Dmitry Gutov 2022-08-30 17:37 ` Eli Zaretskii 2022-08-30 17:39 ` Dmitry Gutov 2022-08-30 17:18 ` Juri Linkov 2022-08-30 7:20 ` Juri Linkov 2022-08-30 11:54 ` Eli Zaretskii 2022-08-30 14:49 ` Sean Whitton 2022-08-30 16:39 ` Juri Linkov 2022-08-30 16:45 ` Eli Zaretskii 2022-08-31 2:38 ` Richard Stallman 2022-08-31 15:48 ` Jonas Bernoulli 2022-08-31 2:38 ` Richard Stallman 2022-09-01 0:32 ` Sean Whitton 2022-09-01 0:33 ` Sean Whitton 2022-08-30 11:56 ` Eli Zaretskii 2022-08-30 12:03 ` Dmitry Gutov 2022-09-01 20:13 ` Dmitry Gutov 2022-08-30 13:25 ` Alfred M. Szmidt 2022-08-30 13:42 ` Po Lu 2022-08-30 14:02 ` Alfred M. Szmidt 2022-10-25 19:22 ` Dmitry Gutov 2022-08-30 14:08 ` Dmitry Gutov 2022-08-30 14:20 ` Alfred M. Szmidt 2022-08-30 14:38 ` Dmitry Gutov 2022-08-30 15:16 ` Stefan Monnier 2022-08-30 15:26 ` Dmitry Gutov 2022-08-30 15:33 ` Lars Ingebrigtsen 2022-08-30 15:45 ` Dmitry Gutov 2022-08-30 15:53 ` Lars Ingebrigtsen 2022-08-30 18:36 ` Stefan Monnier 2022-08-30 20:59 ` Dmitry Gutov 2022-08-30 16:50 ` Eli Zaretskii 2022-08-31 6:51 ` Alfred M. Szmidt 2022-08-30 16:37 ` Eli Zaretskii 2022-08-30 16:45 ` Lars Ingebrigtsen 2022-08-30 17:02 ` Eli Zaretskii 2022-08-30 17:11 ` Lars Ingebrigtsen 2022-08-30 17:34 ` Eli Zaretskii 2022-08-30 17:38 ` Lars Ingebrigtsen 2022-08-30 19:23 ` Sean Whitton 2022-08-30 17:10 ` Dmitry Gutov 2022-08-31 1:28 ` Po Lu 2022-08-31 2:32 ` Dmitry Gutov 2022-08-31 6:53 ` Juri Linkov 2022-08-31 9:55 ` Lars Ingebrigtsen 2022-08-31 10:35 ` Dmitry Gutov 2022-08-31 10:37 ` Lars Ingebrigtsen 2022-08-31 11:09 ` Dmitry Gutov 2022-08-31 15:42 ` Lars Ingebrigtsen 2022-08-31 16:24 ` Dmitry Gutov 2022-08-31 16:39 ` Lars Ingebrigtsen 2022-08-31 18:02 ` Dmitry Gutov 2022-09-01 10:16 ` Lars Ingebrigtsen 2022-09-01 11:31 ` Dmitry Gutov 2022-09-01 11:45 ` Lars Ingebrigtsen 2022-09-01 11:48 ` Dmitry Gutov 2022-09-01 11:57 ` Eli Zaretskii 2022-10-17 0:11 ` Dmitry Gutov 2022-10-17 19:17 ` Juri Linkov 2022-09-01 13:26 ` Robert Pluim 2022-09-01 13:28 ` Dmitry Gutov 2022-09-01 13:32 ` Robert Pluim 2022-09-03 2:51 ` Richard Stallman 2022-09-04 12:27 ` Alfred M. Szmidt 2022-08-31 11:11 ` Dmitry Gutov 2022-08-31 16:06 ` Juri Linkov 2022-08-31 16:21 ` Dmitry Gutov 2022-08-31 16:36 ` Eli Zaretskii 2022-10-16 23:53 ` Dmitry Gutov 2022-10-16 23:54 ` Dmitry Gutov 2022-08-31 16:41 ` Dmitry Gutov 2022-08-31 11:23 ` Eli Zaretskii 2022-08-31 11:57 ` Dmitry Gutov 2022-08-31 12:36 ` Eli Zaretskii 2022-09-04 7:26 ` Philip Kaludercic 2022-09-04 12:27 ` Alfred M. Szmidt 2022-09-04 17:07 ` Philip Kaludercic
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.