log-edit-insert-changelog even when run git/hg from the

log-edit-insert-changelog even when run git/hg from the

[Uwe Brauer]

Hi

There are operations such as rebase, that I cannot run from with vc.

So I use the command line and run 

 git or hg ci

My default editor is emacsclient.

The emacs pops up for the commit message, however it is in text mode.

Moreover in the of hg I use 
!TMP=$($HG root) $HG commit

This way I am sure that emacsclient «is in the directory» I fired the ci
command.

Even when I turn on vc-hg-log-edit-mode
log-edit-insert-changelog does not insert an entry from the ChangeLog
file

So the question is can this achieved somehow?


Thanks and regards

Uwe Brauer 

Re: log-edit-insert-changelog even when run git/hg from the

[Dmitry Gutov]

On 11.09.2022 11:06, Uwe Brauer wrote:
> There are operations such as rebase, that I cannot run from with vc.
> 
> So I use the command line and run
> 
>   git or hg ci
> 
> My default editor is emacsclient.
> 
> The emacs pops up for the commit message, however it is in text mode.
> 
> Moreover in the of hg I use
> !TMP=$($HG root) $HG commit
> 
> This way I am sure that emacsclient «is in the directory» I fired the ci
> command.
> 
> Even when I turn on vc-hg-log-edit-mode
> log-edit-insert-changelog does not insert an entry from the ChangeLog
> file
> 
> So the question is can this achieved somehow?

Have you tried calling it with the prefix argument? Thus enabling the 
USE-FIRST logic.

Re: log-edit-insert-changelog even when run git/hg from the

[Uwe Brauer]

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


> On 11.09.2022 11:06, Uwe Brauer wrote:

> Have you tried calling it with the prefix argument? Thus enabling the
> USE-FIRST logic.

Sure, nothing happens when I do this. Does it work in git?

-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: log-edit-insert-changelog even when run git/hg from the

[Dmitry Gutov]

On 30.09.2022 09:10, Uwe Brauer wrote:
>> On 11.09.2022 11:06, Uwe Brauer wrote:
>> Have you tried calling it with the prefix argument? Thus enabling the
>> USE-FIRST logic.
> Sure, nothing happens when I do this. Does it work in git?

I haven't tried, sorry. But it doesn't sound to be VCS-specific.

My setup is different, and I don't have the time for reproducing this.

I suggest you edebug the command. Perhaps you can learn what needs to be 
changed or fixed there.

Re: log-edit-insert-changelog even when run git/hg from the

[Uwe Brauer]

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

>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:
Date: Fri, 30 Sep 2022 17:48:07 +0200

> On 30.09.2022 09:10, Uwe Brauer wrote:
>>> On 11.09.2022 11:06, Uwe Brauer wrote:
>>> Have you tried calling it with the prefix argument? Thus enabling the
>>> USE-FIRST logic.
>> Sure, nothing happens when I do this. Does it work in git?

> I haven't tried, sorry. But it doesn't sound to be VCS-specific.

> My setup is different, and I don't have the time for reproducing this.

> I suggest you edebug the command. Perhaps you can learn what needs to
> be changed or fixed there.

I did this, first let me remark that the docstring states:

  "Insert a VC commit log message by looking at the ChangeLog.
The idea is to write your ChangeLog entries first, and then use this
command to commit your changes with that log."

So that seems to suggest that only vc-commands are covered but not the ones in which you start the VCS command from the command line and change then emacsclient into vc-hg-log-edit-mode

In any case edebug resulted in 
  (interactive "P")
  (save-excursion
    (let ((eoh (save-excursion (rfc822-goto-eoh) (point))))
      (when (<= (point) eoh)
	(goto-char eoh)
	(if (looking-at "\n") (forward-char 1))))
    (let ((author
	   (let ((log-edit-changelog-use-first
		  (or use-first (eq last-command 'log-edit-insert-changelog))))
	     (log-edit-insert-changelog-entries (log-edit-files)))))
                                                     ^nil

So  log-edit-files return nil!
I tried to debug this as well, 
(defun log-edit-files ()
  "Return the list of files that are about to be committed."
  (ignore-errors (funcall log-edit-listfun)))

but again (funcall log-edit-listfun)))  returned nil.

In a «normal vc-next-action that funcall returns a non nil value


Any suggestions?

-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: log-edit-insert-changelog even when run git/hg from the

[Dmitry Gutov]

On 30.09.2022 18:48, Uwe Brauer wrote:
> I did this, first let me remark that the docstring states:
> 
>    "Insert a VC commit log message by looking at the ChangeLog.
> The idea is to write your ChangeLog entries first, and then use this
> command to commit your changes with that log."
> 
> So that seems to suggest that only vc-commands are covered but not the ones in which you start the VCS command from the command line and change then emacsclient into vc-hg-log-edit-mode
> 
> In any case edebug resulted in
>    (interactive "P")
>    (save-excursion
>      (let ((eoh (save-excursion (rfc822-goto-eoh) (point))))
>        (when (<= (point) eoh)
> 	(goto-char eoh)
> 	(if (looking-at "\n") (forward-char 1))))
>      (let ((author
> 	   (let ((log-edit-changelog-use-first
> 		  (or use-first (eq last-command 'log-edit-insert-changelog))))
> 	     (log-edit-insert-changelog-entries (log-edit-files)))))
>                                                       ^nil
> 
> So  log-edit-files return nil!
> I tried to debug this as well,
> (defun log-edit-files ()
>    "Return the list of files that are about to be committed."
>    (ignore-errors (funcall log-edit-listfun)))
> 
> but again (funcall log-edit-listfun)))  returned nil.
> 
> In a «normal vc-next-action that funcall returns a non nil value
> 
> 
> Any suggestions?

I suppose there are two directions to go from here.

First, you could set log-edit-listfun to a function that retrieves the 
list of files you are currently committing. Not sure how that could be 
implemented, but some reading of files on disk might be involved.

Or maybe Hg passes those files in a comment to the commit edit program, 
I'm not sure.

Alternatively, we could modify log-edit-insert-changelog-entries to 
honor the USE-FIRST argument as well. Perhaps it would see that its 
FILES argument is nil, and when so, if the (newly added) argument 
USE-FIRST is non-nil, it would copy the first entry from the change log.

Re: log-edit-insert-changelog even when run git/hg from the

[Uwe Brauer]

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


> On 30.09.2022 18:48, Uwe Brauer wrote:

> I suppose there are two directions to go from here.

> First, you could set log-edit-listfun to a function that retrieves the
> list of files you are currently committing. Not sure how that could be
> implemented, but some reading of files on disk might be involved.

> Or maybe Hg passes those files in a comment to the commit edit
> program, I'm not sure.

> Alternatively, we could modify log-edit-insert-changelog-entries to
> honor the USE-FIRST argument as well. Perhaps it would see that its
> FILES argument is nil, and when so, if the (newly added) argument
> USE-FIRST is non-nil, it would copy the first entry from the change
> log.


That sounds to me the more natural way. However, I am afraid that seems
to be a corner case and not very many people seem to be interested.

I think the most common use case, that also git users use quite a bit,
would be the rebase command, since in my understanding vc does not
support it and so the ChangeLog in that case would be unavailable.



-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: log-edit-insert-changelog even when run git/hg from the

[Dmitry Gutov]

On 01.10.2022 08:58, Uwe Brauer wrote:
> I think the most common use case, that also git users use quite a bit,
> would be the rebase command, since in my understanding vc does not
> support it and so the ChangeLog in that case would be unavailable.

Wouldn't you need to insert the older entries from ChangeLog, as you're 
rebasing the older commits?

And why are you inserting ChangeLog entries at all? Shouldn't they 
already be in the commit messages to be edited?

I have never used ChangeLog files outside of Emacs development, in any 
project.

But even with Emacs I'm curious about these two questions.

Re: log-edit-insert-changelog even when run git/hg from the

[Uwe Brauer]

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

>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:

> On 01.10.2022 08:58, Uwe Brauer wrote:
>> I think the most common use case, that also git users use quite a bit,
>> would be the rebase command, since in my understanding vc does not
>> support it and so the ChangeLog in that case would be unavailable.

> Wouldn't you need to insert the older entries from ChangeLog, as
> you're rebasing the older commits?

Ok, to give some more details: I don't like rebasing in fact, I hate it, I prefer merging.

However there is a particular workflow, where it comes in handy. Suppose
I develop some code on a branch that I don't push (in hg that would be a
secret branch, as far as I know git does not have phases) or say I
cannot push since I don't have write access. When I finish the other
developers only want to see the final change set.

So I rebase the branch on top of master (in git speech) and collapse all
changesets to one (not sure git has this feature).

In that particular case, I don't want earlier older entries from the ChangeLog.

> And why are you inserting ChangeLog entries at all? Shouldn't they
> already be in the commit messages to be edited?

I use hg, mainly for

    1. latex

    2. emacs

    3. Org mode.

I have some add-ons functions so that add-change-log-entry-other-window
add some specific information about which while and which section or
function was modified. As far as a know there is nothing in vc that
comes close.

And that is the reason why the log-edit-insert-changelog is so useful in my workflow.


BTW, interesting (fun) fact. 

Mercurial and git are often orthogonal in there default setting (like
mercurial hg log always show all branches, if you want only particular
one, you need to specify which git is the other way around, also in
commit: mercurial commits all change set to all files per default but
you can change that by using the interactive option (I think git stage
is something similar).

Now there is a package commit-patch, with fortunately a emacs interace commit-patch-buffer.el 

that allows mercurial users to interactive commit some hunks but not all
(magit has this for a long time I think)

Now the point is that this interface, also it does not rely on
vc-next-action, allows me to use log-edit-insert-changelog.

I have to find out how.



-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: log-edit-insert-changelog even when run git/hg from the

[Dmitry Gutov]

On 01.10.2022 15:51, Uwe Brauer wrote:
>>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> On 01.10.2022 08:58, Uwe Brauer wrote:
>>> I think the most common use case, that also git users use quite a bit,
>>> would be the rebase command, since in my understanding vc does not
>>> support it and so the ChangeLog in that case would be unavailable.
> 
>> Wouldn't you need to insert the older entries from ChangeLog, as
>> you're rebasing the older commits?
> 
> Ok, to give some more details: I don't like rebasing in fact, I hate it, I prefer merging.

I prefer merging as well, but I'm fine with either.

> However there is a particular workflow, where it comes in handy. Suppose
> I develop some code on a branch that I don't push (in hg that would be a
> secret branch, as far as I know git does not have phases) or say I
> cannot push since I don't have write access. When I finish the other
> developers only want to see the final change set.
> 
> So I rebase the branch on top of master (in git speech) and collapse all
> changesets to one (not sure git has this feature).

Normally, when you edit the log message for the combined changeset, all 
the messages from the squashed commits should already be present in the 
buffer. So you would just rearrange them and commit.

So I never felt the need to insert changelog entries again (after doing 
that when creating the original sequence of commits).

> BTW, interesting (fun) fact.
> 
> Mercurial and git are often orthogonal in there default setting (like
> mercurial hg log always show all branches, if you want only particular
> one, you need to specify which git is the other way around, also in
> commit: mercurial commits all change set to all files per default but
> you can change that by using the interactive option (I think git stage
> is something similar).
> 
> Now there is a package commit-patch, with fortunately a emacs interace commit-patch-buffer.el
> 
> that allows mercurial users to interactive commit some hunks but not all
> (magit has this for a long time I think)
> 
> Now the point is that this interface, also it does not rely on
> vc-next-action, allows me to use log-edit-insert-changelog.
> 
> I have to find out how.

We have a ready counterpart for that feature for the Git backend, and I 
should hopefully make it support Hg as well, before Emacs 29 is out.

It does rely on vc-next-action, though (and works when 'C-x v v' is 
called from a diff-mode buffer).

Re: log-edit-insert-changelog even when run git/hg from the

[Uwe Brauer]

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


> On 01.10.2022 15:51, Uwe Brauer wrote:

> I prefer merging as well, but I'm fine with either.


> Normally, when you edit the log message for the combined changeset,
> all the messages from the squashed commits should already be present
> in the buffer. So you would just rearrange them and commit.

Ah, I checked this carefully again, setting up a new dummy repository, did some commits (with ChangeLogs) and I have to admit you are right. I have to check my usecase more carefully. 

> So I never felt the need to insert changelog entries again (after
> doing that when creating the original sequence of commits).


> We have a ready counterpart for that feature for the Git backend, and
> I should hopefully make it support Hg as well, before Emacs 29 is out.

This is really great. But it is not in master, yet?

-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: log-edit-insert-changelog even when run git/hg from the

[Dmitry Gutov]

On 02.10.2022 09:22, Uwe Brauer wrote:
>> On 01.10.2022 15:51, Uwe Brauer wrote:
>> I prefer merging as well, but I'm fine with either.
> 
>> Normally, when you edit the log message for the combined changeset,
>> all the messages from the squashed commits should already be present
>> in the buffer. So you would just rearrange them and commit.
> Ah, I checked this carefully again, setting up a new dummy repository, did some commits (with ChangeLogs) and I have to admit you are right. I have to check my usecase more carefully.
> 
>> So I never felt the need to insert changelog entries again (after
>> doing that when creating the original sequence of commits).
> 
>> We have a ready counterpart for that feature for the Git backend, and
>> I should hopefully make it support Hg as well, before Emacs 29 is out.
> This is really great. But it is not in master, yet?

The Git support is already in master.

Re: log-edit-insert-changelog even when run git/hg from the

[Uwe Brauer]

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


> On 02.10.2022 09:22, Uwe Brauer wrote:

> The Git support is already in master.

Is there any work in progress for mercurial, or is there non enough interest?

-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: log-edit-insert-changelog even when run git/hg from the

[Dmitry Gutov]

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

On 07.10.2022 09:24, Uwe Brauer wrote:
>> On 02.10.2022 09:22, Uwe Brauer wrote:
>> The Git support is already in master.
> Is there any work in progress for mercurial, or is there non enough interest?

Here's a patch I thought almost working, except in the end it doesn't 
move the working revision to the updated branch tip. Any ideas?

[-- Attachment #2: vc-hg-checkin-patch.diff --]
[-- Type: text/x-patch, Size: 1932 bytes --]

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index f4a44df3c2..d5740b6e21 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -1185,19 +1185,33 @@ vc-hg-log-edit-mode
   "Major mode for editing Hg log messages.
 It is based on `log-edit-mode', and has Hg-specific extensions.")
 
+(defvar vc-hg-patch-string nil)
+
 (defun vc-hg-checkin (files comment &optional _rev)
   "Hg-specific version of `vc-backend-checkin'.
 REV is ignored."
-  (let ((amend-extract-fn
-         (lambda (value)
-           (when (equal value "yes")
-             (list "--amend")))))
-    (apply #'vc-hg-command nil 0 files
-           (nconc (list "commit" "-m")
-                  (log-edit-extract-headers `(("Author" . "--user")
-                                              ("Date" . "--date")
-                                              ("Amend" . ,amend-extract-fn))
-                                            comment)))))
+  (apply #'vc-hg-command nil 0 files
+         (nconc (list "commit"
+                      "-m")
+                (vc-hg--extract-headers comment))))
+
+(defun vc-hg-checkin-patch (patch-string comment)
+  (let ((patch-file (make-temp-file "hg-patch")))
+    (with-temp-file patch-file
+      (insert patch-string))
+    (unwind-protect
+        (apply #'vc-hg-command nil 0 nil
+               (nconc (list "import" "--bypass" patch-file "-m")
+                      (vc-hg--extract-headers comment)))
+      (delete-file patch-file))))
+
+(defun vc-hg--extract-headers (comment)
+  (log-edit-extract-headers `(("Author" . "--user")
+                              ("Date" . "--date")
+                              ("Amend" . (lambda (value)
+                                           (when (equal value "yes")
+                                             (list "--amend")))))
+                            comment))
 
 (defun vc-hg-find-revision (file rev buffer)
   (let ((coding-system-for-read 'binary)

Re: log-edit-insert-changelog even when run git/hg from the

[Uwe Brauer]

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

>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:

> On 07.10.2022 09:24, Uwe Brauer wrote:
>>> On 02.10.2022 09:22, Uwe Brauer wrote:
>>> The Git support is already in master.
>> Is there any work in progress for mercurial, or is there non enough interest?

> Here's a patch I thought almost working, except in the end it doesn't 
> move the working revision to the updated branch tip. Any ideas?

I will try to check this later the day. I pulled but cannot find the changeset your patch must be applied to. 

Can you please enlighten me?

thanks



> diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
> index f4a44df3c2..d5740b6e21 100644

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: log-edit-insert-changelog even when run git/hg from the

[Dmitry Gutov]

On 10.10.2022 09:39, Uwe Brauer wrote:
>>>> "DG" == Dmitry Gutov<dgutov@yandex.ru>  writes:
>> On 07.10.2022 09:24, Uwe Brauer wrote:
>>>> On 02.10.2022 09:22, Uwe Brauer wrote:
>>>> The Git support is already in master.
>>> Is there any work in progress for mercurial, or is there non enough interest?
>> Here's a patch I thought almost working, except in the end it doesn't
>> move the working revision to the updated branch tip. Any ideas?
> I will try to check this later the day. I pulled but cannot find the changeset your patch must be applied to.
> 
> Can you please enlighten me?

Just 'git apply' on top of the latest master.

[Serious BUG in master 93136169cba] (was: log-edit-insert-changelog even when run git/hg from the)

[Uwe Brauer]

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


> On 10.10.2022 09:39, Uwe Brauer wrote:

> Just 'git apply' on top of the latest master.

Thanks, there seems a serious BUG.

My steps

The latest commit is 
93136169cba9c7c8f827dbc2f579b95a75d15fad


    1) I apply your patch.
    2) I run ./configure --prefix=/opt/emacs29 --with-x-toolkit=athena --without-pop --with-mailutils
    3) I run make bootstrap
    4) Make
    5) cd src
    6) ./emacs
    7) I obtain two screens I attach

Button line it might be athena, I will try to recompile without athena

But I cannot send even a BUG report from the new version!

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [Serious BUG in master 93136169cba]

[Uwe Brauer]

[-- Attachment #1.1: Type: text/plain, Size: 927 bytes --]

>>> "UB" == Uwe Brauer <oub@mat.ucm.es> writes:

>> On 10.10.2022 09:39, Uwe Brauer wrote:

>> Just 'git apply' on top of the latest master.

> Thanks, there seems a serious BUG.

> My steps

> The latest commit is 
> 93136169cba9c7c8f827dbc2f579b95a75d15fad


>     1) I apply your patch.
>     2) I run ./configure --prefix=/opt/emacs29 --with-x-toolkit=athena --without-pop --with-mailutils
>     3) I run make bootstrap
>     4) Make
>     5) cd src
>     6) ./emacs
>     7) I obtain two screens I attach

> Button line it might be athena, I will try to recompile without athena

> But I cannot send even a BUG report from the new version!


I forgot to attach the screenshots


-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #1.2: emacs-windows1.png --]
[-- Type: image/png, Size: 84007 bytes --]

[-- Attachment #1.3: emacs-windows.png --]
[-- Type: image/png, Size: 265883 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

[Confirmed] (was: [Serious BUG in master 93136169cba])

[Uwe Brauer]

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

>>> "UB" == Uwe Brauer <oub@mat.ucm.es> writes:

>> On 10.10.2022 09:39, Uwe Brauer wrote:

>> Just 'git apply' on top of the latest master.

> Thanks, there seems a serious BUG.

> My steps

> The latest commit is 
> 93136169cba9c7c8f827dbc2f579b95a75d15fad


>     1) I apply your patch.
>     2) I run ./configure --prefix=/opt/emacs29 --with-x-toolkit=athena --without-pop --with-mailutils
>     3) I run make bootstrap
>     4) Make
>     5) cd src
>     6) ./emacs
>     7) I obtain two screens I attach

> Button line it might be athena, I will try to recompile without athena

> But I cannot send even a BUG report from the new version!

Confirmed it is the option --with-x-toolkit=athena

That causes the problem. 

I will submit a bug report

-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

[patch: first impression] (was: log-edit-insert-changelog even when run git/hg from the)

[Uwe Brauer]

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


> On 10.10.2022 09:39, Uwe Brauer wrote:

> Just 'git apply' on top of the latest master.

I have to wait till the BUG I reported is resolved, since otherwise working with Emacs is an ordeal. However I have given it a try and it seems, that in mercurial speech, 
there is a 

 hg up tip 

Missing, the @ is not on tip (I think called HEAD in git), but on the changeset (commit) just before.

I have to debug this in order to see what is going on.

The behavior is a bit annoying especially if you continue working and want to commit again, in which case you might create a new head (in mercurial speech), if you are not carefully checking the graph before.

But if you do, your solution is  somehow working. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression] (was: log-edit-insert-changelog even when run git/hg from the)

[Dmitry Gutov]

On 10.10.2022 17:53, Uwe Brauer wrote:
>> On 10.10.2022 09:39, Uwe Brauer wrote:
>> Just 'git apply' on top of the latest master.
> I have to wait till the BUG I reported is resolved, since otherwise working with Emacs is an ordeal. However I have given it a try and it seems, that in mercurial speech,
> there is a
> 
>   hg up tip
> 
> Missing, the @ is not on tip (I think called HEAD in git), but on the changeset (commit) just before.
> 
> I have to debug this in order to see what is going on.
> 
> The behavior is a bit annoying especially if you continue working and want to commit again, in which case you might create a new head (in mercurial speech), if you are not carefully checking the graph before.
> 
> But if you do, your solution is  somehow working.

OK. What if our solution also automatically calls `hg up tip` at the 
end? Should that always work, or would it mess up situations where the 
user actually wanted to be in that state (head not at tip), and was in 
that originally?

E.g. if they wanted to "create a new head" using the new method to 
commit a patch?

Re: [patch: first impression]

[Uwe Brauer]

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

>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:

> On 10.10.2022 17:53, Uwe Brauer wrote:
>>> On 10.10.2022 09:39, Uwe Brauer wrote:
>>> Just 'git apply' on top of the latest master.
>> I have to wait till the BUG I reported is resolved, since otherwise
>> working with Emacs is an ordeal. However I have given it a try and
>> it seems, that in mercurial speech,
>> there is a
>> hg up tip
>> Missing, the @ is not on tip (I think called HEAD in git), but on
>> the changeset (commit) just before.
>> I have to debug this in order to see what is going on.
>> The behavior is a bit annoying especially if you continue working
>> and want to commit again, in which case you might create a new head
>> (in mercurial speech), if you are not carefully checking the graph
>> before.
>> But if you do, your solution is  somehow working.

> OK. What if our solution also automatically calls `hg up tip` at the
> end? Should that always work, or would it mess up situations where the
> user actually wanted to be in that state (head not at tip), and was in
> that originally?

Let me think a bit about it and maybe ask other mercurial users on the
hg mailing list. I would say hg up tip should be ok, since this the
default behavior anyway

> E.g. if they wanted to "create a new head" using the new method to
> commit a patch?

I am not sure you know but in mercurial you could create an anonymous 
head (which is impossible in git, well at least not permanently) but it
is also strongly avised against doing it, so if you want to do that you
would create before the commit a named branch, topic or maybe a
bookmark.

But let me ask first others

-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

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

On 10.10.2022 18:41, Uwe Brauer wrote:
>>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> On 10.10.2022 17:53, Uwe Brauer wrote:
>>>> On 10.10.2022 09:39, Uwe Brauer wrote:
>>>> Just 'git apply' on top of the latest master.
>>> I have to wait till the BUG I reported is resolved, since otherwise
>>> working with Emacs is an ordeal. However I have given it a try and
>>> it seems, that in mercurial speech,
>>> there is a
>>> hg up tip
>>> Missing, the @ is not on tip (I think called HEAD in git), but on
>>> the changeset (commit) just before.
>>> I have to debug this in order to see what is going on.
>>> The behavior is a bit annoying especially if you continue working
>>> and want to commit again, in which case you might create a new head
>>> (in mercurial speech), if you are not carefully checking the graph
>>> before.
>>> But if you do, your solution is  somehow working.
> 
>> OK. What if our solution also automatically calls `hg up tip` at the
>> end? Should that always work, or would it mess up situations where the
>> user actually wanted to be in that state (head not at tip), and was in
>> that originally?
> 
> Let me think a bit about it and maybe ask other mercurial users on the
> hg mailing list. I would say hg up tip should be ok, since this the
> default behavior anyway

Another option is to use shelving.

See the attached slightly different patch.

>> E.g. if they wanted to "create a new head" using the new method to
>> commit a patch?
> 
> I am not sure you know but in mercurial you could create an anonymous
> head (which is impossible in git, well at least not permanently) but it
> is also strongly avised against doing it, so if you want to do that you
> would create before the commit a named branch, topic or maybe a
> bookmark.

It's probably the same as "detached HEAD" in Git.

[-- Attachment #2: vc-hg-checkin-patch-with-shelve.diff --]
[-- Type: text/x-patch, Size: 2170 bytes --]

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index f4a44df3c2..30c1453e42 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -1185,19 +1185,38 @@ vc-hg-log-edit-mode
   "Major mode for editing Hg log messages.
 It is based on `log-edit-mode', and has Hg-specific extensions.")
 
+(defvar vc-hg-patch-string nil)
+
 (defun vc-hg-checkin (files comment &optional _rev)
   "Hg-specific version of `vc-backend-checkin'.
 REV is ignored."
-  (let ((amend-extract-fn
-         (lambda (value)
-           (when (equal value "yes")
-             (list "--amend")))))
-    (apply #'vc-hg-command nil 0 files
-           (nconc (list "commit" "-m")
-                  (log-edit-extract-headers `(("Author" . "--user")
-                                              ("Date" . "--date")
-                                              ("Amend" . ,amend-extract-fn))
-                                            comment)))))
+  (apply #'vc-hg-command nil 0 files
+         (nconc (list "commit"
+                      "-m")
+                (vc-hg--extract-headers comment))))
+
+(defun vc-hg-checkin-patch (patch-string comment)
+  (let ((patch-file (make-temp-file "hg-patch"))
+        (shelf-name (make-temp-name "vc-checkin-patch")))
+    (with-temp-file patch-file
+      (insert patch-string))
+    (vc-hg-command nil t nil "shelve" "-d" shelf-name)
+    (vc-hg-command nil 0 nil "shelve" "-n" shelf-name)
+    (unwind-protect
+        (apply #'vc-hg-command nil 0 nil
+               (nconc (list "import" patch-file "-m")
+                      (vc-hg--extract-headers comment)))
+      (progn
+        (vc-hg-command nil 0 nil "unshelve" "-n" shelf-name)
+        (delete-file patch-file)))))
+
+(defun vc-hg--extract-headers (comment)
+  (log-edit-extract-headers `(("Author" . "--user")
+                              ("Date" . "--date")
+                              ("Amend" . (lambda (value)
+                                           (when (equal value "yes")
+                                             (list "--amend")))))
+                            comment))
 
 (defun vc-hg-find-revision (file rev buffer)
   (let ((coding-system-for-read 'binary)

Re: [patch: first impression]

[Uwe Brauer]

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


> On 10.10.2022 18:41, Uwe Brauer wrote:

> Another option is to use shelving.

Do you mean: the not committed hunks are shelved? My first reaction is:
that is counter intuitive (and commit-patch does not do this neither,
but commit patch is not very VC orientated anyhow).

But now that you mention it, mercurial also supports interactive
shelving (so to speak the inverse operation to interactive commit. I am
not sure that git has that feature).

So couldn't that also be included? That would be really great, since commit-patch does not support it.

The binding could be either C-u vc-next-action or 
something like this (just to explain the idea)

(defun toggle-commit-or-shelve-interactively ()
  (interactive)
  (with-output-to-temp-buffer "vc-list-buffer"
    (princ "List of functions\n")
    (princ "1: commit interactively\n")					  
    (princ "2: shelv interactively \n"))
    (let  ((ch (string-to-char (read-string "Which choice: 1: 2:  "))))
      (call-interactively (cond ((eql ch ?1) #'vc-commit-interactively)					  
				((eql ch ?2) #'vc-shelve-interactively)
				(t (error 'args-out-of-range '(1 2  ch))))))
    (kill-buffer "vc-list-buffer"))



> See the attached slightly different patch.

I will try to test this later (I presume I don't need to recompile the
whole emacs 😉; just kidding)

> It's probably the same as "detached HEAD" in Git.

Right the difference is, that, in my understanding, this HEAD will be
delete in the next garbage collection, while in mercurial it will stay..


> diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
> index f4a44df3c2..30c1453e42 100644
> --- a/lisp/vc/vc-hg.el
> +++ b/lisp/vc/vc-hg.el
> @@ -1185,19 +1185,38 @@ vc-hg-log-edit-mode
>    "Major mode for editing Hg log messages.
>  It is based on `log-edit-mode', and has Hg-specific extensions.")
 
> +(defvar vc-hg-patch-string nil)
> +
>  (defun vc-hg-checkin (files comment &optional _rev)
>    "Hg-specific version of `vc-backend-checkin'.
>  REV is ignored."
> -  (let ((amend-extract-fn
> -         (lambda (value)
> -           (when (equal value "yes")
> -             (list "--amend")))))
> -    (apply #'vc-hg-command nil 0 files
> -           (nconc (list "commit" "-m")
> -                  (log-edit-extract-headers `(("Author" . "--user")
> -                                              ("Date" . "--date")
> -                                              ("Amend" . ,amend-extract-fn))
> -                                            comment)))))
> +  (apply #'vc-hg-command nil 0 files
> +         (nconc (list "commit"
> +                      "-m")
> +                (vc-hg--extract-headers comment))))
> +
> +(defun vc-hg-checkin-patch (patch-string comment)
> +  (let ((patch-file (make-temp-file "hg-patch"))
> +        (shelf-name (make-temp-name "vc-checkin-patch")))
> +    (with-temp-file patch-file
> +      (insert patch-string))
> +    (vc-hg-command nil t nil "shelve" "-d" shelf-name)
> +    (vc-hg-command nil 0 nil "shelve" "-n" shelf-name)
> +    (unwind-protect
> +        (apply #'vc-hg-command nil 0 nil
> +               (nconc (list "import" patch-file "-m")
> +                      (vc-hg--extract-headers comment)))
> +      (progn
> +        (vc-hg-command nil 0 nil "unshelve" "-n" shelf-name)
> +        (delete-file patch-file)))))
> +
> +(defun vc-hg--extract-headers (comment)
> +  (log-edit-extract-headers `(("Author" . "--user")
> +                              ("Date" . "--date")
> +                              ("Amend" . (lambda (value)
> +                                           (when (equal value "yes")
> +                                             (list "--amend")))))
> +                            comment))
 
>  (defun vc-hg-find-revision (file rev buffer)
>    (let ((coding-system-for-read 'binary)


-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

On 11.10.2022 09:25, Uwe Brauer wrote:
> Do you mean: the not committed hunks are shelved?

Shelved before the commit is made and restored at the end, see the patch.

> But now that you mention it, mercurial also supports interactive
> shelving (so to speak the inverse operation to interactive commit. I am
> not sure that git has that feature).
> 
> So couldn't that also be included? That would be really great, since commit-patch does not support it.
> 
> The binding could be either C-u vc-next-action or
> something like this (just to explain the idea)
> 
> (defun toggle-commit-or-shelve-interactively ()
>    (interactive)
>    (with-output-to-temp-buffer "vc-list-buffer"
>      (princ "List of functions\n")
>      (princ "1: commit interactively\n")					
>      (princ "2: shelv interactively \n"))
>      (let  ((ch (string-to-char (read-string "Which choice: 1: 2:  "))))
>        (call-interactively (cond ((eql ch ?1) #'vc-commit-interactively)					
> 				((eql ch ?2) #'vc-shelve-interactively)
> 				(t (error 'args-out-of-range '(1 2  ch))))))
>      (kill-buffer "vc-list-buffer"))

I'm not sure where to plug functionality like that.

> I will try to test this later (I presume I don't need to recompile the
> whole emacs 😉; just kidding)

FWIW, you don't really have to recompile Emacs. You could `C-M-x` on the 
new definitions instead after applying either of the patches.

Re: [patch: first impression]

[Uwe Brauer]

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

>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:

> On 11.10.2022 09:25, Uwe Brauer wrote:
>> Do you mean: the not committed hunks are shelved?

> Shelved before the commit is made and restored at the end, see the patch.

Ok, I see may this is safer than just to run hg up tip after the commit
because hg might complain that there are uncommitted changes. But
shelving is also a bit slow, (well I can only talk about GNU emacs,
which is a huge repository and it is converted via the hg-git plugin,
may other hg users might have more insight)

In any case I have to run a series of tests to say something more
useful, right now, I am more or less forced to bisect GNU emacs because
there is a serious GTK bug which makes the latest master unusable for
me. So I might answer only by tomorrow....


>> But now that you mention it, mercurial also supports interactive
>> shelving (so to speak the inverse operation to interactive commit. I am
>> not sure that git has that feature).
>> So couldn't that also be included? That would be really great, since
>> commit-patch does not support it.
>> The binding could be either C-u vc-next-action or
>> something like this (just to explain the idea)
>> (defun toggle-commit-or-shelve-interactively ()
>> (interactive)
>> (with-output-to-temp-buffer "vc-list-buffer"
>> (princ "List of functions\n")
>> (princ "1: commit interactively\n")					
>> (princ "2: shelv interactively \n"))
>> (let  ((ch (string-to-char (read-string "Which choice: 1: 2:  "))))
>> (call-interactively (cond ((eql ch ?1) #'vc-commit-interactively)					
>> ((eql ch ?2) #'vc-shelve-interactively)
>> (t (error 'args-out-of-range '(1 2  ch))))))
>> (kill-buffer "vc-list-buffer"))

> I'm not sure where to plug functionality like that.

I see, but supporting interactively shelving seems like a nice feature
to have, don't you think?



>> I will try to test this later (I presume I don't need to recompile the
>> whole emacs 😉; just kidding)

> FWIW, you don't really have to recompile Emacs. You could `C-M-x` on
> the new definitions instead after applying either of the patches.

Thanks again for all your efforts.


-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

On 11.10.2022 15:24, Uwe Brauer wrote:
>> I'm not sure where to plug functionality like that.
> I see, but supporting interactively shelving seems like a nice feature
> to have, don't you think?

I suppose it could work the same as 'git stash' integration in vc-git.

As you can see, it doesn't take too much code, so someone interested is 
welcome to implement the corresponding feature for vc-hg.

Re: [patch: first impression]

[Uwe Brauer]

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

>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:

> On 11.10.2022 15:24, Uwe Brauer wrote:
>>> I'm not sure where to plug functionality like that.
>> I see, but supporting interactively shelving seems like a nice feature
>> to have, don't you think?

> I suppose it could work the same as 'git stash' integration in vc-git.

I thought git stash is more like hg strip, but I have to check the hg-git rosetta

> As you can see, it doesn't take too much code, so someone interested
> is welcome to implement the corresponding feature for vc-hg.

Right 😉

Once your patch in included, I will try it then.

-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

On 11.10.2022 19:55, Uwe Brauer wrote:
>>>> "DG" == Dmitry Gutov<dgutov@yandex.ru>  writes:
>> On 11.10.2022 15:24, Uwe Brauer wrote:
>>>> I'm not sure where to plug functionality like that.
>>> I see, but supporting interactively shelving seems like a nice feature
>>> to have, don't you think?
>> I suppose it could work the same as 'git stash' integration in vc-git.
> I thought git stash is more like hg strip, but I have to check the hg-git rosetta
> 
>> As you can see, it doesn't take too much code, so someone interested
>> is welcome to implement the corresponding feature for vc-hg.
> Right 😉
> 
> Once your patch in included, I will try it then.

I have just pushed a backend-agnostic implementation to master which 
should hopefully avoid all those problems at the cost of some runtime 
overhead (file copying and such).

Please try it out whenever convenient.

Re: [patch: first impression]

[Dr. Arne Babenhauserheide]

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


Dmitry Gutov <dgutov@yandex.ru> writes:
> I have just pushed a backend-agnostic implementation to master which
> should hopefully avoid all those problems at the cost of some runtime
> overhead (file copying and such).
>
> Please try it out whenever convenient.

Thank you for your work on this!

(just wanting to give some kudos: I am very excited to see this arrive,
but I could not test it yet due to being swamped with other things)

Best wishes,
Arne
-- 
Unpolitisch sein
heißt politisch sein,
ohne es zu merken.
draketo.de

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 1125 bytes --]

Re: [patch: first impression]

[Uwe Brauer]

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


> On 11.10.2022 19:55, Uwe Brauer wrote:

> I have just pushed a backend-agnostic implementation to master which
> should hopefully avoid all those problems at the cost of some runtime
> overhead (file copying and such).

> Please try it out whenever convenient.

Thanks, I shall do.

I also want to mention, that, besides Arne, there is one more HG user/developer who would like to test.

I did test both patches, and the external commit-patch on the GNU emacs repository, that I converted to HG. Conclusion your second patch using shelve and unshelve is considerably slower than your first patch and commit-patch (which I think was to be expected).

BTW, looking at your code, what is actually happen is that a temporary patch file generated, that is manipulated and later committed, that is why I am not sure that hg shelve -i
or git stash interactively could be easily implemented.


Thanks again for your efforts

Regards

Uwe 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

On 17.10.2022 08:48, Dr. Arne Babenhauserheide wrote:
> Dmitry Gutov<dgutov@yandex.ru>  writes:
>> I have just pushed a backend-agnostic implementation to master which
>> should hopefully avoid all those problems at the cost of some runtime
>> overhead (file copying and such).
>>
>> Please try it out whenever convenient.
> Thank you for your work on this!
> 
> (just wanting to give some kudos: I am very excited to see this arrive,
> but I could not test it yet due to being swamped with other things)

Welcome!

But props to Juri first of all, who took the initiative and done the 
majority of the work, including the best-performing vc-git implementation.

Re: [patch: first impression]

[Dmitry Gutov]

On 17.10.2022 10:11, Uwe Brauer wrote:
> BTW, looking at your code, what is actually happen is that a temporary patch file generated, that is manipulated and later committed, that is why I am not sure that hg shelve -i
> or git stash interactively could be easily implemented.

The problem with using 'hg shelve' or 'git stash' for implementing this 
is that either can create a merge conflict when restoring the previous 
changes on top of the committed patch.

And we don't want to force the user to have to resolve them manually 
every time.

Re: [patch: first impression]

[Uwe Brauer]

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

>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:

> On 17.10.2022 10:11, Uwe Brauer wrote:
>> BTW, looking at your code, what is actually happen is that a
>> temporary patch file generated, that is manipulated and later
>> committed, that is why I am not sure that hg shelve -i
>> or git stash interactively could be easily implemented.

> The problem with using 'hg shelve' or 'git stash' for implementing
> this is that either can create a merge conflict when restoring the
> previous changes on top of the committed patch.

> And we don't want to force the user to have to resolve them manually
> every time.

Very true, I did not think about it when I proposed the feature

-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Uwe Brauer]

[-- Attachment #1.1: Type: text/plain, Size: 764 bytes --]


> On 11.10.2022 19:55, Uwe Brauer wrote:

> I have just pushed a backend-agnostic implementation to master which
> should hopefully avoid all those problems at the cost of some runtime
> overhead (file copying and such).

> Please try it out whenever convenient.

This time I obtain an error I attach. I have to say that I am currently
using 7bf17ceee8c2d347917541e143ce25609e90ebbb, because the actual
master version gave me some error. With that version I could
successfully test your first 2 patches, but now it fails
I am puzzled

-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #1.2: vc-bug --]
[-- Type: application/octet-stream, Size: 4141 bytes --]

Debugger entered--Lisp error: (file-missing "Opening input file" "No such file or directory" "/tmp/vc-checkin-patchN6scd0/src/xterm.c")
  copy-file("/tmp/vc-checkin-patchN6scd0/src/xterm.c" "/home/oub/src/emacs-hg/src/xterm.c" t)
  #f(compiled-function () #<bytecode -0x9ca690101c46bae>)()
  vc-default-checkin-patch(Hg #("diff --git a/src/xfns.c b/src/xfns.c\n--- a/src/xfn..." 0 41 (face diff-header fontified t) 41 53 (face (diff-file-header diff-header) fontified t) 53 58 (face diff-header fontified t) 58 70 (face (diff-file-header diff-header) fontified t) 70 71 (face diff-header fontified t) 71 93 (face diff-hunk-header fontified t) 93 94 (fontified t) 94 150 (face diff-context fontified t) 150 151 (face diff-indicator-removed fontified t) 151 152 (face diff-removed fontified t) 152 153 (face diff-indicator-removed fontified t) 153 196 (face diff-removed fontified t) 196 197 (face diff-indicator-removed fontified t) 197 269 (face diff-removed fontified t) 269 270 (face diff-indicator-removed fontified t) 270 327 (face diff-removed fontified t) ...) #("Author: test commit\n" 0 6 (pabbrev-added t face log-edit-header fontified nil field header) 6 7 (face log-edit-header fontified nil field header) 7 8 (fontified nil field header) 8 19 (face log-edit-header fontified nil field header) 19 20 (fontified nil)))
  apply(vc-default-checkin-patch Hg (#("diff --git a/src/xfns.c b/src/xfns.c\n--- a/src/xfn..." 0 41 (face diff-header fontified t) 41 53 (face (diff-file-header diff-header) fontified t) 53 58 (face diff-header fontified t) 58 70 (face (diff-file-header diff-header) fontified t) 70 71 (face diff-header fontified t) 71 93 (face diff-hunk-header fontified t) 93 94 (fontified t) 94 150 (face diff-context fontified t) 150 151 (face diff-indicator-removed fontified t) 151 152 (face diff-removed fontified t) 152 153 (face diff-indicator-removed fontified t) 153 196 (face diff-removed fontified t) 196 197 (face diff-indicator-removed fontified t) 197 269 (face diff-removed fontified t) 269 270 (face diff-indicator-removed fontified t) 270 327 (face diff-removed fontified t) ...) #("Author: test commit\n" 0 6 (pabbrev-added t face log-edit-header fontified nil field header) 6 7 (face log-edit-header fontified nil field header) 7 8 (fontified nil field header) 8 19 (face log-edit-header fontified nil field header) 19 20 (fontified nil))))
  vc-call-backend(Hg checkin-patch #("diff --git a/src/xfns.c b/src/xfns.c\n--- a/src/xfn..." 0 41 (face diff-header fontified t) 41 53 (face (diff-file-header diff-header) fontified t) 53 58 (face diff-header fontified t) 58 70 (face (diff-file-header diff-header) fontified t) 70 71 (face diff-header fontified t) 71 93 (face diff-hunk-header fontified t) 93 94 (fontified t) 94 150 (face diff-context fontified t) 150 151 (face diff-indicator-removed fontified t) 151 152 (face diff-removed fontified t) 152 153 (face diff-indicator-removed fontified t) 153 196 (face diff-removed fontified t) 196 197 (face diff-indicator-removed fontified t) 197 269 (face diff-removed fontified t) 269 270 (face diff-indicator-removed fontified t) 270 327 (face diff-removed fontified t) ...) #("Author: test commit\n" 0 6 (pabbrev-added t face log-edit-header fontified nil field header) 6 7 (face log-edit-header fontified nil field header) 7 8 (fontified nil field header) 8 19 (face log-edit-header fontified nil field header) 19 20 (fontified nil)))
  #f(compiled-function (files comment) #<bytecode -0x1afedb8b59403496>)((#("src/xfns.c" 0 10 (fontified t face (diff-file-header diff-header))) #("src/xterm.c" 0 11 (fontified t face (diff-file-header diff-header)))) #("Author: test commit\n" 0 6 (pabbrev-added t face log-edit-header fontified nil field header) 6 7 (face log-edit-header fontified nil field header) 7 8 (fontified nil field header) 8 19 (face log-edit-header fontified nil field header) 19 20 (fontified nil)))
  vc-finish-logentry()
  funcall-interactively(vc-finish-logentry)
  call-interactively(vc-finish-logentry)
  log-edit-done()
  funcall-interactively(log-edit-done)
  call-interactively(log-edit-done nil nil)
  command-execute(log-edit-done)

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

On 17.10.2022 23:44, Uwe Brauer wrote:
>> On 11.10.2022 19:55, Uwe Brauer wrote:
>> I have just pushed a backend-agnostic implementation to master which
>> should hopefully avoid all those problems at the cost of some runtime
>> overhead (file copying and such).
>> Please try it out whenever convenient.
> This time I obtain an error I attach. I have to say that I am currently
> using 7bf17ceee8c2d347917541e143ce25609e90ebbb, because the actual
> master version gave me some error. With that version I could
> successfully test your first 2 patches, but now it fails
> I am puzzled

Thanks for testing, please try again with the latest master, I pushed a 
fix in 5e77265523.

Or maybe try with 7bf17ceee8c2d, with patches 4a6ad6e18f and 5e77265523 
on top of it.

Re: [patch: first impression]

[Uwe Brauer]

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


> On 17.10.2022 23:44, Uwe Brauer wrote:

> Thanks for testing, please try again with the latest master, I pushed
> a fix in 5e77265523.

> Or maybe try with 7bf17ceee8c2d, with patches 4a6ad6e18f and
> 5e77265523 on top of it.

Thanks, your latest commit  works with commit 7bf17ceee8c2d.

Some benchmarks.

I cloned emacs master with hg 

|     | repo size (GB) |
| Git |            1.4 |
| HG  |            1.8 |

I am using for this experiment master 0954689cb

Steps:

    - hg src/xfns.c    -r b9aff5fdb89092b68 (git reset (not sure hard or soft)

    - hg src/xterm.c    -r b9aff5fdb89092b68

    - vc-dir (mark the two files)

    - vc-diff

    - delete hunks

    - run vc-next-action: time: 36 sec on my X1 Thinkpad 2017

    - do the same but run commit-patch-buffer using the perl script
      commit-patch
      time: 26 sec 

Conclusion, for most practical purpose the lisp implementation is
enough. I just considered an extreme case.

Thanks and regards



-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Uwe Brauer]

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

>>> "UB" == Uwe Brauer <oub@mat.ucm.es> writes:

>> On 17.10.2022 23:44, Uwe Brauer wrote:

>> Thanks for testing, please try again with the latest master, I pushed
>> a fix in 5e77265523.

>> Or maybe try with 7bf17ceee8c2d, with patches 4a6ad6e18f and
>> 5e77265523 on top of it.

> Thanks, your latest commit  works with commit 7bf17ceee8c2d.

> Some benchmarks.

> I cloned emacs master with hg 

> |     | repo size (GB) |
> | Git |            1.4 |
> | HG  |            1.8 |

> I am using for this experiment master 0954689cb

> Steps:

>     - hg src/xfns.c    -r b9aff5fdb89092b68 (git reset (not sure hard or soft)

>     - hg src/xterm.c    -r b9aff5fdb89092b68

>     - vc-dir (mark the two files)

>     - vc-diff

>     - delete hunks

>     - run vc-next-action: time: 36 sec on my X1 Thinkpad 2017

>     - do the same but run commit-patch-buffer using the perl script
>       commit-patch
>       time: 26 sec 

> Conclusion, for most practical purpose the lisp implementation is
> enough. I just considered an extreme case.

I just realised that after finishing, the repository contains the files 
 
xterm.c.orig
xfns.c.orig

I can easily get rid of them by running 

 hg purge (or git clean -fd)

But what is the purpose of leaving these files, is this by design or a
side effect of your implementation.


-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

On 18.10.2022 09:33, Uwe Brauer wrote:
>      - do the same but run commit-patch-buffer using the perl script
>        commit-patch
>        time: 26 sec

That's all after pressing C-c C-c after typing in the commit message, right?

That's a lot. I have a big checkout of the Mozilla repo, and a similar 
operation takes about ~3 seconds. And that already feels sluggish.

But of course it depends on the number of files and their sizes, I suppose.

> Conclusion, for most practical purpose the lisp implementation is
> enough. I just considered an extreme case.

Wish we managed to implement something faster for Hg.

'hg import --bypass' seemed the most promising option, but the 'hg 
update tip' step that you suggested can end up in conflict.

I've searched for some "plumbing" Hg command corresponding to 'git reset 
--soft' and couldn't find it.

Re: [patch: first impression]

[Dmitry Gutov]

On 18.10.2022 15:23, Uwe Brauer wrote:
> I just realised that after finishing, the repository contains the files
>   
> xterm.c.orig
> xfns.c.orig
> 
> I can easily get rid of them by running
> 
>   hg purge (or git clean -fd)
> 
> But what is the purpose of leaving these files, is this by design or a
> side effect of your implementation.

Hmm, I see it too.

That is something the "--no-backup-if-mismatch" argument was supposed to 
avoid.

And here it apparently performs backups even in the absence of mismatches.

Re: [patch: first impression]

[Uwe Brauer]

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

>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:

> On 18.10.2022 09:33, Uwe Brauer wrote:
>> - do the same but run commit-patch-buffer using the perl script
>> commit-patch
>> time: 26 sec

> That's all after pressing C-c C-c after typing in the commit message, right?

Right!

> That's a lot. I have a big checkout of the Mozilla repo, and a similar
> operation takes about ~3 seconds. And that already feels sluggish.

Well 

    1. it is a huge repository,

    2. It has over 200 000 commits

    3. It is converted.

So I did the same on the hg repository itself. Which 

    1. Only 200 MB

    2. 50 000 commits

    3. It is native hg

I did the same, reverted to files edit the diff and then commited

    1. Lisp implementation 8 sec

    2. Commit-patch 4 sec


Not sure what is the conclusion here. Well in general hg is slower especially if there are a lot of named branches, but the emacs-hg repository has one the default
and hg has 2, default and stable

Strange indeed.




> But of course it depends on the number of files and their sizes, I suppose.

>> Conclusion, for most practical purpose the lisp implementation is
>> enough. I just considered an extreme case.

> Wish we managed to implement something faster for Hg.

> 'hg import --bypass' seemed the most promising option, but the 'hg
> update tip' step that you suggested can end up in conflict.


It is not clear to me why?


> I've searched for some "plumbing" Hg command corresponding to 'git
> reset --soft' and couldn't find it.

 git reset --soft
 should be 

  hg revert 

if I am not mistaken

 git reset --hard 
would be 
hg revert --no-backup

I think





-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

On 18.10.2022 16:28, Uwe Brauer wrote:

> I did the same, reverted to files edit the diff and then commited
> 
>      1. Lisp implementation 8 sec
> 
>      2. Commit-patch 4 sec
> 
> 
> Not sure what is the conclusion here. Well in general hg is slower especially if there are a lot of named branches, but the emacs-hg repository has one the default
> and hg has 2, default and stable
> 
> Strange indeed.

Try this modification:

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index df51f52bc7..3e78b8cfe9 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1715,9 +1715,6 @@ vc-default-checkin-patch
                                               "--no-backup-if-mismatch"
                                               "-i" "-"))
                (user-error "Patch failed: %s" (buffer-string))))
-          (dolist (f files)
-            (with-current-buffer (get-file-buffer f)
-              (revert-buffer t t t)))
            (vc-call-backend backend 'checkin files comment))
        (dolist (f files)
          (copy-file (expand-file-name f tmpdir)


This step is probably unnecessary, and might take up extra time.

>> But of course it depends on the number of files and their sizes, I suppose.
> 
>>> Conclusion, for most practical purpose the lisp implementation is
>>> enough. I just considered an extreme case.
> 
>> Wish we managed to implement something faster for Hg.
> 
>> 'hg import --bypass' seemed the most promising option, but the 'hg
>> update tip' step that you suggested can end up in conflict.
> 
> 
> It is not clear to me why?

The patch you might be committing can have changes conflicting with the 
existing modifications inside the same files. We decided to allow that 
(because diff-mode definitely permits such modification).

But 'hg update', when the file has certain changes in the same area that 
should be updated with the switch between revisions, drops you into 
merge mode.

>> I've searched for some "plumbing" Hg command corresponding to 'git
>> reset --soft' and couldn't find it.
> 
>   git reset --soft
>   should be
> 
>    hg revert
> 
> if I am not mistaken

No, 'hg revert' is the same as 'git checkout FILE' or 'git restore 
FILE'. It affects the file contents.

Whereas 'git reset --soft' only switches the current HEAD to a different 
revision, without touching the file contents on disk (those outside of 
.git directory, of course).

Re: [patch: first impression]

[Uwe Brauer]

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


> On 18.10.2022 16:28, Uwe Brauer wrote:

> Try this modification:

> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
> index df51f52bc7..3e78b8cfe9 100644
> --- a/lisp/vc/vc.el
> +++ b/lisp/vc/vc.el
> @@ -1715,9 +1715,6 @@ vc-default-checkin-patch
>                                               "--no-backup-if-mismatch"
>                                               "-i" "-"))
>                (user-error "Patch failed: %s" (buffer-string))))
> -          (dolist (f files)
> -            (with-current-buffer (get-file-buffer f)
> -              (revert-buffer t t t)))
>            (vc-call-backend backend 'checkin files comment))
>        (dolist (f files)
>          (copy-file (expand-file-name f tmpdir)


> This step is probably unnecessary, and might take up extra time.


I will 

> The patch you might be committing can have changes conflicting with
> the existing modifications inside the same files. We decided to allow
> that (because diff-mode definitely permits such modification).

Oh that was not clear to me.

> But 'hg update', when the file has certain changes in the same area
> that should be updated with the switch between revisions, drops you
> into merge mode.


> No, 'hg revert' is the same as 'git checkout FILE' or 'git restore
> FILE'. It affects the file contents.

> Whereas 'git reset --soft' only switches the current HEAD to a
> different revision, without touching the file contents on disk (those
> outside of .git directory, of course).

I am confused now! That description sounds like 
hg update 

to me
but it seems git reset --soft would not call a merge in the case you
describe

what's about 


hg update -C 

Or is that more like git reset --hard?

then?

Maybe some of the hg gurus that are on the CC and know git could
enlighten us?

@Arne, @Georges?



-- 
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

On 18.10.2022 18:38, Uwe Brauer wrote:
> I am confused now! That description sounds like
> hg update
> 
> to me
> but it seems git reset --soft would not call a merge in the case you
> describe

Right.

> what's about
> 
> 
> hg update -C
> 
> Or is that more like git reset --hard?

Yes, 'hg update -C' is like 'git reset --hard', it that it discards 
uncommitted changes. 'hg update' tries to merge them instead. Just like 
'git checkout' might do. Either way, they generally change file contents 
on disk.

'git reset --soft' (or just 'git reset') doesn't touch file contents on 
disk.

Re: [patch: first impression]

[Uwe Brauer]

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

>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:

> On 18.10.2022 18:38, Uwe Brauer wrote:
>> I am confused now! That description sounds like
>> hg update
>> to me
>> but it seems git reset --soft would not call a merge in the case you
>> describe

> Right.

>> what's about
>> hg update -C
>> Or is that more like git reset --hard?

> Yes, 'hg update -C' is like 'git reset --hard', it that it discards
> uncommitted changes. 'hg update' tries to merge them instead. Just
> like 'git checkout' might do. Either way, they generally change file
> contents on disk.

> 'git reset --soft' (or just 'git reset') doesn't touch file contents
> on disk.

What's about 

 hg uncommit 

Or 

Not sure 
hg strip --keep -r .

But strip is usually slow.

BTW I can't see much difference in your latest patch, I have to test it more, the emacs-hg is very slow even from the command line

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

On 18.10.2022 19:47, Uwe Brauer wrote:
>>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> On 18.10.2022 18:38, Uwe Brauer wrote:
>>> I am confused now! That description sounds like
>>> hg update
>>> to me
>>> but it seems git reset --soft would not call a merge in the case you
>>> describe
> 
>> Right.
> 
>>> what's about
>>> hg update -C
>>> Or is that more like git reset --hard?
> 
>> Yes, 'hg update -C' is like 'git reset --hard', it that it discards
>> uncommitted changes. 'hg update' tries to merge them instead. Just
>> like 'git checkout' might do. Either way, they generally change file
>> contents on disk.
> 
>> 'git reset --soft' (or just 'git reset') doesn't touch file contents
>> on disk.
> 
> What's about
> 
>   hg uncommit

Again, it changes file contents on disk.

> Or
> 
> Not sure
> hg strip --keep -r .

Not sure what this one does, but the description doesn't sound relevant.

Is it more like 'git cherry-pick'?

> But strip is usually slow.
> 
> BTW I can't see much difference in your latest patch, I have to test it more, the emacs-hg is very slow even from the command line

That's unfortunate.

Re: [patch: first impression]

[Dr. Arne Babenhauserheide]

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


Dmitry Gutov <dgutov@yandex.ru> writes:

>>>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:
>>> On 18.10.2022 18:38, Uwe Brauer wrote:
>>>> I am confused now! That description sounds like
>>>> hg update
>>>> to me
>>>> but it seems git reset --soft would not call a merge in the case you
>>>> describe

The goal is to make the modified files stay exactly as they are, right?

For that you can use the merge tool that always uses the local changes:

hg import --bypass <patch> && hg update --merge --tool internal:merge-local
(see hg help mergetool for details)

⇒ import the patch, then update and in case of conflict, use the local
  version.

To test:

hg init foo
cd foo
echo first > 1
hg add 1
hg ci -m first
echo interactive.patch > 1
hg diff > interactive.patch
echo second > 1
hg import --bypass interactive.patch -m "interactive.patch"
hg update --merge --tool internal:merge-local
hg diff

Result:
diff --git a/1 b/1
--- a/1
+++ b/1
@@ -1,1 +1,1 @@
-interactive.patch
+second

Best wishes,
Arne
-- 
Unpolitisch sein
heißt politisch sein,
ohne es zu merken.
draketo.de

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 1125 bytes --]

Re: [patch: first impression]

[Dr. Arne Babenhauserheide]

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


"Dr. Arne Babenhauserheide" <arne_bab@web.de> writes:

> hg import --bypass <patch> && hg update --merge --tool internal:merge-local
cleaner with tip (in case there are multiple heads):

hg import --bypass <patch> && hg update --merge --tool internal:merge-local tip

Best wishes,
Arne
-- 
Unpolitisch sein
heißt politisch sein,
ohne es zu merken.
draketo.de

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 1125 bytes --]

Re: [patch: first impression]

[Uwe Brauer]

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

>>> "AB" == Arne Babenhauserheide <arne_bab@web.de> writes:

> Dmitry Gutov <dgutov@yandex.ru> writes:

>>>>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:
>>> On 18.10.2022 18:38, Uwe Brauer wrote:
>>>>> I am confused now! That description sounds like
>>>>> hg update
>>>>> to me
>>>>> but it seems git reset --soft would not call a merge in the case you
>>>>> describe

> The goal is to make the modified files stay exactly as they are, right?

> For that you can use the merge tool that always uses the local changes:

> hg import --bypass <patch> && hg update --merge --tool internal:merge-local
> (see hg help mergetool for details)

> ⇒ import the patch, then update and in case of conflict, use the local
>   version.

Thanks, Arne, 

Dmitry, with this approach maybe then even 
hg shelve -i
would be doable?

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

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

On 18.10.2022 22:29, Dr. Arne Babenhauserheide wrote:
> "Dr. Arne Babenhauserheide"<arne_bab@web.de>  writes:
> 
>> hg import --bypass <patch> && hg update --merge --tool internal:merge-local
> cleaner with tip (in case there are multiple heads):
> 
> hg import --bypass <patch> && hg update --merge --tool internal:merge-local tip

Thanks!.

For best compatibility with existing behavior (default and vc-git's), it 
seems I was looking for the merge tool 'internal:local'.

Could either of you test the attached patch?

It should be both faster and a little safer than using the default impl.

[-- Attachment #2: vc-hg-checkin-patch-with-mergetool-local.diff --]
[-- Type: text/x-patch, Size: 1959 bytes --]

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index 2eebe2d543..30b4e55818 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -1189,16 +1189,32 @@ vc-hg-log-edit-mode
 (defun vc-hg-checkin (files comment &optional _rev)
   "Hg-specific version of `vc-backend-checkin'.
 REV is ignored."
-  (let ((amend-extract-fn
-         (lambda (value)
-           (when (equal value "yes")
-             (list "--amend")))))
-    (apply #'vc-hg-command nil 0 files
-           (nconc (list "commit" "-m")
-                  (log-edit-extract-headers `(("Author" . "--user")
-                                              ("Date" . "--date")
-                                              ("Amend" . ,amend-extract-fn))
-                                            comment)))))
+  (apply #'vc-hg-command nil 0 files
+         (nconc (list "commit"
+                      "-m")
+                (vc-hg--extract-headers comment))))
+
+(defun vc-hg-checkin-patch (patch-string comment)
+  (let ((patch-file (make-temp-file "hg-patch")))
+    (write-region patch-string nil patch-file)
+    (unwind-protect
+        (progn
+          (apply #'vc-hg-command nil 0 nil
+                 (nconc (list "import" "--bypass" patch-file "-m")
+                        (vc-hg--extract-headers comment)))
+          (vc-hg-command nil 0 nil
+                         "update"
+                         "--merge" "--tool" "internal:local"
+                         "tip"))
+      (delete-file patch-file))))
+
+(defun vc-hg--extract-headers (comment)
+  (log-edit-extract-headers `(("Author" . "--user")
+                              ("Date" . "--date")
+                              ("Amend" . (lambda (value)
+                                           (when (equal value "yes")
+                                             (list "--amend")))))
+                            comment))
 
 (defun vc-hg-find-revision (file rev buffer)
   (let ((coding-system-for-read 'binary)

Re: [patch: first impression]

[Dmitry Gutov]

On 19.10.2022 08:40, Uwe Brauer wrote:
> Dmitry, with this approach maybe then even
> hg shelve -i
> would be doable?

I suppose something like this could work for Git and Hg:

1) Stash/Shelve all current changes.
2) Apply the provided patch. Somehow deal carefully with the new files 
in there, especially when said files are already present (and 
unregistered) in the work tree. Probably by scanning the file list in 
the patch in advance and 'git add'-ing all the existing new file first 
in step 1.
3) Stash the currently applied new changes. Also don't forget to 'git 
add' any unregistered files.
4) Unstash/unshelve the previous stash.

And I guess it will require a new key binding in diff-mode buffers.

Re: [patch: first impression]

[Uwe Brauer]

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

>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:

> On 19.10.2022 08:40, Uwe Brauer wrote:
>> Dmitry, with this approach maybe then even
>> hg shelve -i
>> would be doable?

> I suppose something like this could work for Git and Hg:

> 1) Stash/Shelve all current changes.
> 2) Apply the provided patch. Somehow deal carefully with the new files
> in there, especially when said files are already present (and
> unregistered) in the work tree. Probably by scanning the file list in
> the patch in advance and 'git add'-ing all the existing new file first
> in step 1.
> 3) Stash the currently applied new changes. Also don't forget to 'git
> add' any unregistered files.
> 4) Unstash/unshelve the previous stash.

That sounds sensible to me.


> And I guess it will require a new key binding in diff-mode buffers.

I see. BTW I only realised by now that the fact that 
a buffer in diff-mode is in read-only mode is not essential but not a
required feature. 

I say this, because sometimes the diff file contains a lot of hunks and
I only want to commit 2 of say 20.

Then toggling read-only off and just delete the hunks via kill-region,
simplify things a lot (instead of killing each hunk separately, using
next-hunk and kill-hunk).



-- 
Warning: Content may be disturbing to some audiences
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 
https://addons.thunderbird.net/en-US/thunderbird/addon/gmail-conversation-view/

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Uwe Brauer]

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

>>> "DG" == Dmitry Gutov <dgutov@yandex.ru> writes:

> On 18.10.2022 22:29, Dr. Arne Babenhauserheide wrote:
>> "Dr. Arne Babenhauserheide"<arne_bab@web.de>  writes:
>> 
>>> hg import --bypass <patch> && hg update --merge --tool internal:merge-local
>> cleaner with tip (in case there are multiple heads):
>> 
>> hg import --bypass <patch> && hg update --merge --tool internal:merge-local tip

> Thanks!.

> For best compatibility with existing behavior (default and vc-git's), it 
> seems I was looking for the merge tool 'internal:local'.

> Could either of you test the attached patch?

Sure, the attached diff is against the actual master I presume?


> It should be both faster and a little safer than using the default impl.

> diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
> index 2eebe2d543..30b4e55818 100644
> --- a/lisp/vc/vc-hg.el
> +++ b/lisp/vc/vc-hg.el
> @@ -1189,16 +1189,32 @@ vc-hg-log-edit-mode
>  (defun vc-hg-checkin (files comment &optional _rev)
>    "Hg-specific version of `vc-backend-checkin'.
>  REV is ignored."
> -  (let ((amend-extract-fn
> -         (lambda (value)
> -           (when (equal value "yes")
> -             (list "--amend")))))
> -    (apply #'vc-hg-command nil 0 files
> -           (nconc (list "commit" "-m")
> -                  (log-edit-extract-headers `(("Author" . "--user")
> -                                              ("Date" . "--date")
> -                                              ("Amend" . ,amend-extract-fn))
> -                                            comment)))))
> +  (apply #'vc-hg-command nil 0 files
> +         (nconc (list "commit"
> +                      "-m")
> +                (vc-hg--extract-headers comment))))
> +
> +(defun vc-hg-checkin-patch (patch-string comment)
> +  (let ((patch-file (make-temp-file "hg-patch")))
> +    (write-region patch-string nil patch-file)
> +    (unwind-protect
> +        (progn
> +          (apply #'vc-hg-command nil 0 nil
> +                 (nconc (list "import" "--bypass" patch-file "-m")
> +                        (vc-hg--extract-headers comment)))
> +          (vc-hg-command nil 0 nil
> +                         "update"
> +                         "--merge" "--tool" "internal:local"
> +                         "tip"))
> +      (delete-file patch-file))))
> +
> +(defun vc-hg--extract-headers (comment)
> +  (log-edit-extract-headers `(("Author" . "--user")
> +                              ("Date" . "--date")
> +                              ("Amend" . (lambda (value)
> +                                           (when (equal value "yes")
> +                                             (list "--amend")))))
> +                            comment))
 
>  (defun vc-hg-find-revision (file rev buffer)
>    (let ((coding-system-for-read 'binary)


-- 
Warning: Content may be disturbing to some audiences
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 
https://addons.thunderbird.net/en-US/thunderbird/addon/gmail-conversation-view/

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

On 24.10.2022 18:34, Uwe Brauer wrote:
>>>> "DG" == Dmitry Gutov<dgutov@yandex.ru>  writes:
>> On 18.10.2022 22:29, Dr. Arne Babenhauserheide wrote:
>>> "Dr. Arne Babenhauserheide"<arne_bab@web.de>   writes:
>>>
>>>> hg import --bypass <patch> && hg update --merge --tool internal:merge-local
>>> cleaner with tip (in case there are multiple heads):
>>>
>>> hg import --bypass <patch> && hg update --merge --tool internal:merge-local tip
>> Thanks!.
>> For best compatibility with existing behavior (default and vc-git's), it
>> seems I was looking for the merge tool 'internal:local'.
>> Could either of you test the attached patch?
> Sure, the attached diff is against the actual master I presume?
> 
> 

Yup.

Re: [patch: first impression]

[Dmitry Gutov]

On 24.10.2022 18:32, Uwe Brauer wrote:
> I see. BTW I only realised by now that the fact that
> a buffer in diff-mode is in read-only mode is not essential but not a
> required feature.

Yes. And you can also edit the diff before typing 'C-x v v'.

> I say this, because sometimes the diff file contains a lot of hunks and
> I only want to commit 2 of say 20.
> 
> Then toggling read-only off and just delete the hunks via kill-region,
> simplify things a lot (instead of killing each hunk separately, using
> next-hunk and kill-hunk).

If might be handy to add commands that act on region (and delete full 
hunks, for instance), so that you aren't forced to toggle read-only.

Re: [patch: first impression]

[Uwe Brauer]

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


> On 18.10.2022 22:29, Dr. Arne Babenhauserheide wrote:

> Thanks!.

> For best compatibility with existing behavior (default and vc-git's), it 
> seems I was looking for the merge tool 'internal:local'.

> Could either of you test the attached patch?

> It should be both faster and a little safer than using the default impl.

It is indeed. Here is how I tested it, after pulling the latest master.

    - hg revert -r  a2dd9d683a02 (git hash), xterm.c and xfns.c are changed (reverted to that change set)

    - vc-dir

    - vc-diff

    - delete the first 2 hunks in xterm.c and then in xfns.c

    - vc-next-action

    - write a message

    - commit: commit took 10 sec!!,

    - I did the same step with the perl script commit-patch: 14 sec

So that looks indeed much faster! Thanks a lot 

Can somebody else please test this?

regards


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Robert Pluim]

>>>>> On Mon, 24 Oct 2022 20:28:20 +0300, Dmitry Gutov <dgutov@yandex.ru> said:

    Dmitry> If might be handy to add commands that act on region (and delete full
    Dmitry> hunks, for instance), so that you aren't forced to toggle read-only.

Deleting hunks is already supported:

    k runs the command diff-hunk-kill (found in diff-mode-shared-map),
    which is an interactive byte-compiled Lisp function in `diff-mode.el'.

    It is bound to k and M-k.

    (diff-hunk-kill)

    Kill the hunk at point.

Extending that to support a prefix count arg and the region shouldnʼt
be too hard.

obert
-- 

Re: [patch: first impression]

[Uwe Brauer]

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

>>> "RP" == Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Mon, 24 Oct 2022 20:28:20 +0300, Dmitry Gutov <dgutov@yandex.ru> said:
Dmitry> If might be handy to add commands that act on region (and delete full
Dmitry> hunks, for instance), so that you aren't forced to toggle read-only.

> Deleting hunks is already supported:

>     k runs the command diff-hunk-kill (found in diff-mode-shared-map),
>     which is an interactive byte-compiled Lisp function in `diff-mode.el'.

>     It is bound to k and M-k.

>     (diff-hunk-kill)

>     Kill the hunk at point.

Thanks, I think I mentioned that

> Extending that to support a prefix count arg and the region shouldnʼt
> be too hard.

I am not sure that prefix count would be that useful (I wouldn't count
hunk before deleting them in mass. It sounds a bit like a keyboard
macro)

Deleting regions would  be nice, however

> obert

-- 
Warning: Content may be disturbing to some audiences
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 
https://addons.thunderbird.net/en-US/thunderbird/addon/gmail-conversation-view/

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Robert Pluim]

>>>>> On Tue, 25 Oct 2022 11:14:27 +0200, Uwe Brauer <oub@mat.ucm.es> said:

    >> Extending that to support a prefix count arg and the region shouldnʼt
    >> be too hard.

    Uwe> I am not sure that prefix count would be that useful (I wouldn't count
    Uwe> hunk before deleting them in mass. It sounds a bit like a keyboard
    Uwe> macro)

Hmm, yes. Iʼd probably just welly on 'k' in that situation

    Uwe> Deleting regions would  be nice, however

Patches welcome :-)

Robert
-- 

Re: [patch: first impression]

[Eli Zaretskii]

> From: Uwe Brauer <oub@mat.ucm.es>
> Cc: Dmitry Gutov <dgutov@yandex.ru>, Uwe Brauer <oub@mat.ucm.es>,
>  "Dr. Arne Babenhauserheide" <arne_bab@web.de>,
>  georges.racinet@octobus.net, emacs-devel@gnu.org
> Date: Tue, 25 Oct 2022 11:14:27 +0200
> 
> > Deleting hunks is already supported:
> 
> >     k runs the command diff-hunk-kill (found in diff-mode-shared-map),
> >     which is an interactive byte-compiled Lisp function in `diff-mode.el'.
> 
> >     It is bound to k and M-k.
> 
> >     (diff-hunk-kill)
> 
> >     Kill the hunk at point.
> 
> Thanks, I think I mentioned that
> 
> > Extending that to support a prefix count arg and the region shouldnʼt
> > be too hard.
> 
> I am not sure that prefix count would be that useful (I wouldn't count
> hunk before deleting them in mass. It sounds a bit like a keyboard
> macro)

I think we should support an argument to mean the count of hunks to
delete.  And negative arguments should kill backward.  Every Emacs
command that kills does (or at least should) behave like that, and I
think this is a nice and convenient consistency.

> Deleting regions would  be nice, however

Only complete hunks in the region, I guess?  Deleting partial hunks
will make the diffs invalid.

Re: [patch: first impression]

[Uwe Brauer]

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



> I think we should support an argument to mean the count of hunks to
> delete.  And negative arguments should kill backward.  Every Emacs
> command that kills does (or at least should) behave like that, and I
> think this is a nice and convenient consistency.

I know that every kill command does that, I just wonder how often this is still used.

> Only complete hunks in the region, I guess?  Deleting partial hunks
> will make the diffs invalid.


Yes of course!
-- 
Warning: Content may be disturbing to some audiences
I strongly condemn Putin's war of aggression against the Ukraine.
I support to deliver weapons to Ukraine's military. 
I support the ban of Russia from SWIFT.
I support the EU membership of the Ukraine. 
https://addons.thunderbird.net/en-US/thunderbird/addon/gmail-conversation-view/

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5673 bytes --]

Re: [patch: first impression]

[Dmitry Gutov]

On 24.10.2022 20:53, Uwe Brauer wrote:
>      - write a message
> 
>      - commit: commit took 10 sec!!,
> 
>      - I did the same step with the perl script commit-patch: 14 sec

Excellent, thank for testing. :-)

> So that looks indeed much faster! Thanks a lot
> 
> Can somebody else please test this?

I tested it a little myself before sending the patch, so it seems good 
to go. Now installed on master.