GNU bug report logs -
#64055
31.0.50; log-view-modify-change-comment support for Git and Hg
Previous Next
To reply to this bug, email your comments to 64055 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 13 Jun 2023 23:05:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Morgan Smith <Morgan.J.Smith <at> outlook.com>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Tue, 13 Jun 2023 23:05:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello!
[0001-enable-editing-log-comments.patch (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]
Please look at this patch. Ignore the bad commit message for now, I'm
trying to figure out how to edit that :P.
I've spent far too long looking at git documentation and source code. I
think this is the best way to enable this feature. In fact I really
don't think there is another way to implement this feature that doesn't
rely on the "autosquash" git feature.
I found the hack located in vc-git-modify-change-comment' of setting
"GIT_SEQUENCE_EDITOR=:" on the internet and I don't understand how it
works.
Also I'm not really sure what the 'vc-dispatcher-browsing' function is
supposed to do but it's getting in my way. Like are 'diff-mode' buffers
a "directory browser buffer"? I don't understand the intent here.
Also you'll noticed I deleted some comments in vc.el. That's mainly to
highlight them so we can talk about them now. Is there a good way to
stop users from editing remote commits?
I feel like I should probably just keep working on this and get it more
polished and answer some of my own questions before posting here but I'm
getting kind of frustrated with this. I am many hours deep into this
problem.
Thanks,
Morgan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Wed, 14 Jun 2023 08:01:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 64055 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Tue, 13 Jun 2023 18:59:24 -0400, Morgan Smith <Morgan.J.Smith <at> outlook.com> said:
Morgan> Please look at this patch. Ignore the bad commit message for now, I'm
Morgan> trying to figure out how to edit that :P.
Morgan> I've spent far too long looking at git documentation and source code. I
Morgan> think this is the best way to enable this feature. In fact I really
Morgan> don't think there is another way to implement this feature that doesn't
Morgan> rely on the "autosquash" git feature.
Does 'git commit --amend -m' not work?
Morgan> I found the hack located in vc-git-modify-change-comment' of setting
Morgan> "GIT_SEQUENCE_EDITOR=:" on the internet and I don't understand how it
Morgan> works.
Morgan> Also I'm not really sure what the 'vc-dispatcher-browsing' function is
Morgan> supposed to do but it's getting in my way. Like are 'diff-mode' buffers
Morgan> a "directory browser buffer"? I don't understand the intent here.
Morgan> Also you'll noticed I deleted some comments in vc.el. That's mainly to
Morgan> highlight them so we can talk about them now. Is there a good way to
Morgan> stop users from editing remote commits?
You canʼt stop anyone from doing anything on their local system, but I
guess you could check if 'git merge-base' is upstream.
Morgan> I feel like I should probably just keep working on this and get it more
Morgan> polished and answer some of my own questions before posting here but I'm
Morgan> getting kind of frustrated with this. I am many hours deep into this
Morgan> problem.
We can be your rubber duck :-)
Robert
--
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Wed, 14 Jun 2023 11:42:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Robert Pluim <rpluim <at> gmail.com> writes:
> Does 'git commit --amend -m' not work?
This only works if you want to amend the latest commit. If you want to
edit commit messages of older commits generally one would use the reword
feature during an interactive rebase. To do everything from the
commandline I think we need to use the autosquash feature.
'git commit --amend' is actually already built into the Emacs vc system
and works great. I don't think you can use it without a diff though.
> Robert
Morgan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Wed, 14 Jun 2023 13:14:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 64055 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Wed, 14 Jun 2023 07:41:18 -0400, Morgan Smith <Morgan.J.Smith <at> outlook.com> said:
Morgan> Robert Pluim <rpluim <at> gmail.com> writes:
>> Does 'git commit --amend -m' not work?
Morgan> This only works if you want to amend the latest commit. If you want to
Morgan> edit commit messages of older commits generally one would use the reword
Morgan> feature during an interactive rebase. To do everything from the
Morgan> commandline I think we need to use the autosquash feature.
Yes, eg magitʼs interactive rebase is great for that kind of stuff. I donʼt
know offhand if it uses autosquash.
Morgan> 'git commit --amend' is actually already built into the Emacs vc system
Morgan> and works great. I don't think you can use it without a diff though.
Fixing that would fit 99.99% of the cases where I want to reword a
commit message, and I suspect Iʼm not alone.
Robert
--
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Wed, 14 Jun 2023 13:55:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Robert Pluim <rpluim <at> gmail.com> writes:
> Yes, eg magitʼs interactive rebase is great for that kind of stuff. I donʼt
> know offhand if it uses autosquash.
I look briefly at their code and they use a perl wrapper around the
interactive rebase. I suppose that's another way of doing it. I don't
think it's any better or worse then my approach though.
>
> Morgan> 'git commit --amend' is actually already built into the Emacs vc system
> Morgan> and works great. I don't think you can use it without a diff though.
>
> Fixing that would fit 99.99% of the cases where I want to reword a
> commit message, and I suspect Iʼm not alone.
Strange. I'm usually working on at least a couple commits at a time.
For example, if I'm working on a change and I notice a typo in a
docstring unrelated to my change then I'll stick that into a commit.
Then I'll fix up all my commit messages at the end of my session.
Regardless, the current amend logic is great if you need to amend code
changes. For the case without a diff, you could simply use the feature
I'm adding here.
Morgan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Wed, 14 Jun 2023 15:31:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 64055 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Wed, 14 Jun 2023 09:54:20 -0400, Morgan Smith <Morgan.J.Smith <at> outlook.com> said:
Morgan> Robert Pluim <rpluim <at> gmail.com> writes:
>> Yes, eg magitʼs interactive rebase is great for that kind of stuff. I donʼt
>> know offhand if it uses autosquash.
Morgan> I look briefly at their code and they use a perl wrapper around the
Morgan> interactive rebase. I suppose that's another way of doing it. I don't
Morgan> think it's any better or worse then my approach though.
OK
>>
Morgan> 'git commit --amend' is actually already built into the Emacs vc system
Morgan> and works great. I don't think you can use it without a diff though.
>>
>> Fixing that would fit 99.99% of the cases where I want to reword a
>> commit message, and I suspect Iʼm not alone.
Morgan> Strange. I'm usually working on at least a couple commits at a time.
Morgan> For example, if I'm working on a change and I notice a typo in a
Morgan> docstring unrelated to my change then I'll stick that into a commit.
Morgan> Then I'll fix up all my commit messages at the end of my session.
That kind of thing I just leave in, and if it gets in the way Iʼll
stash it, and then commit it afterwards. But then again, for Emacs I
try not to have too many commits in flight at the same time. To each
their own.
Morgan> Regardless, the current amend logic is great if you need to amend code
Morgan> changes. For the case without a diff, you could simply use the feature
Morgan> I'm adding here.
Right
Robert
--
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sat, 17 Jun 2023 02:41:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hi!
This is a welcome addition, thanks for your efforts.
On 14/06/2023 01:59, Morgan Smith wrote:
> - "-1" "--no-color" ,@(ensure-list vc-git-log-switches)
> + "-1" "--no-color"
> + ;; The same as the default "medium" format but it doesn't
> + ;; put spaces at the beginning of the body. This is so
> + ;; we can grab this as the initial value when calling
> + ;; log-view-modify-change-comment
> + "--pretty=format:commit %H%nAuthor: %an %ae%nDate: %ad%n%n%B"
> + ,@(ensure-list vc-git-log-switches)
Does this improve the overall look of the log? Otherwise, we could
remove the extra spaces when grabbing the log message (as long as we
know how many were added), just like we do in log-edit-insert-changelog.
But an even better solution, perhaps, would be to delegate to
(vc-call-backend backend 'expanded-log-entry revision).
Here's how it looked in my old, unfinished version of this feature:
(defun log-view-modify-change-comment ()
"Edit the change comment displayed at point."
(interactive)
(let* ((file (and log-view-per-file-logs
(log-view-current-file)))
(revision (log-view-current-tag))
(backend (vc-responsible-backend (or file (car
log-view-vc-fileset)))))
(vc-modify-change-comment (list file)
revision
(vc-call-backend backend
'expanded-log-entry revision))))
It will need to fall back to log-view-extract-comment for backends that
don't have this method defined, though. Such as CVS/RCS/SVN/SCCS.
> -;; - Third, doing message editing in log-view might be a natural way to go
> -;; about it, but editing any but the last commit (and even it, if it's
> -;; been pushed) is a dangerous operation in Git, which we shouldn't make
> -;; too easy for users to perform.
...
> I found the hack located in vc-git-modify-change-comment' of setting
> "GIT_SEQUENCE_EDITOR=:" on the internet and I don't understand how it
> works.
Apparently ":' is a shell built-in which "does nothing and then
succeeds" (https://stackoverflow.com/a/29094904). "true" should also work.
> Also I'm not really sure what the 'vc-dispatcher-browsing' function is
> supposed to do but it's getting in my way. Like are 'diff-mode' buffers
> a "directory browser buffer"?
The docstring is 15 years old and never updated. If we can find a better
wording, that would be great.
> I don't understand the intent here.
Check out the two places where it is called.
> Also you'll noticed I deleted some comments in vc.el. That's mainly to
> highlight them so we can talk about them now. Is there a good way to
> stop users from editing remote commits?
We could do something like this: take the remote branch associated with
the current branch, and its top commit. And then, somehow, see if the
commit to be edited is reachable by traversing up history.
There likely are some known Git snippets out there on SO or general
Internet that do these steps, but I haven't looked yet.
Anyway, we could do this check first thing inside
vc-git-modify-change-comment and, when the operation is dangerous,
doubly prompt the user whether they want to proceed.
Or, a more involved approach: add a new backend function which would do
the check. Then log-view-modify-change-comment could abort earlier.
> I feel like I should probably just keep working on this and get it more
> polished and answer some of my own questions before posting here but I'm
> getting kind of frustrated with this. I am many hours deep into this
> problem.
Please feel free to ask questions.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 01 Oct 2024 13:53:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 64055 <at> debbugs.gnu.org (full text, mbox):
On 01/10/2024 05:37, Sean Whitton wrote:
> The difficulty of prompting is that the prompt has to be short to fit in
> the echo area. But something like
>
> This commit is published; are you sure you want to rewrite history?
>
> is only explanatory to someone who already has a lot of git-specific
> knowledge. So, by default, we I think we should refuse to rewrite any
> commit that does not appear in*vc-outgoing*.
Yep! That sounds good to me.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 01 Oct 2024 15:24:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Wed 14 Jun 2023 at 03:13pm +02, Robert Pluim wrote:
> Morgan> 'git commit --amend' is actually already built into the Emacs vc system
> Morgan> and works great. I don't think you can use it without a diff though.
>
> Fixing that would fit 99.99% of the cases where I want to reword a
> commit message, and I suspect Iʼm not alone.
Like Morgan, I regularly rebase solely to edit commit messages other
than that of the most recent commit.
Morgan, Dmitry, you have both posted WIP on this. Have you made any
progress? Are you interested in looking at it again?
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 01 Oct 2024 15:25:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sat 17 Jun 2023 at 05:40am +03, Dmitry Gutov wrote:
> We could do something like this: take the remote branch associated with the
> current branch, and its top commit. And then, somehow, see if the commit to be
> edited is reachable by traversing up history.
>
> There likely are some known Git snippets out there on SO or general Internet
> that do these steps, but I haven't looked yet.
>
> Anyway, we could do this check first thing inside vc-git-modify-change-comment
> and, when the operation is dangerous, doubly prompt the user whether they want
> to proceed.
I think that we refuse to proceed, or at least ask for confirmation,
in the case that the commit the user wants to edit does not appear in
*vc-outgoing* (C-x v O).
The difficulty of prompting is that the prompt has to be short to fit in
the echo area. But something like
This commit is published; are you sure you want to rewrite history?
is only explanatory to someone who already has a lot of git-specific
knowledge. So, by default, we I think we should refuse to rewrite any
commit that does not appear in *vc-outgoing*.
Then there could be a defcustom to enable a yes/no prompt (or to enable
unconditionally rewriting). In the docstring for the defcustom we could
provide a reference to the relevant git documentation, so the user can
find out what this notion of "rewriting history" is all about.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 01 Oct 2024 19:33:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 64055 <at> debbugs.gnu.org (full text, mbox):
On 01/10/2024 05:38, Sean Whitton wrote:
> Morgan, Dmitry, you have both posted WIP on this. Have you made any
> progress? Are you interested in looking at it again?
I haven't worked on it, sorry.
If my old WIP looks useful, you're quite welcome to use it as a starting
point. No need for attribution or anything like that.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Wed, 02 Oct 2024 00:02:02 GMT)
Full text and
rfc822 format available.
Message #38 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Tue 01 Oct 2024 at 10:32pm +03, Dmitry Gutov wrote:
> On 01/10/2024 05:38, Sean Whitton wrote:
>> Morgan, Dmitry, you have both posted WIP on this. Have you made any
>> progress? Are you interested in looking at it again?
>
> I haven't worked on it, sorry.
>
> If my old WIP looks useful, you're quite welcome to use it as a starting
> point. No need for attribution or anything like that.
Could you point me at that WIP, please? It's not posted to this bug and
I couldn't find it in my mail archives.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Wed, 02 Oct 2024 23:22:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 64055 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 02/10/2024 03:01, Sean Whitton wrote:
> On Tue 01 Oct 2024 at 10:32pm +03, Dmitry Gutov wrote:
>
>> On 01/10/2024 05:38, Sean Whitton wrote:
>>> Morgan, Dmitry, you have both posted WIP on this. Have you made any
>>> progress? Are you interested in looking at it again?
>> I haven't worked on it, sorry.
>>
>> If my old WIP looks useful, you're quite welcome to use it as a starting
>> point. No need for attribution or anything like that.
> Could you point me at that WIP, please? It's not posted to this bug and
> I couldn't find it in my mail archives.
Yeah, sorry about that.
Here's the diff I have lying around from back then. Not sure how much it
will help - the changes in log-view.el seems like an improvement (more
generic approach), whereas the vc-git-modify-change-comment definition
might be better in Morgan's patch (it /would/ be nice to be able to edit
older commits, not just the most recent one).
[log-view-modify-change-comment.diff (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Thu, 10 Oct 2024 02:40:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
In Dmitry's patch he takes the approach of calling the
expanded-log-entry backend function to get the message to edit.
This is not a real VC backend function -- in fact it's a log-view
feature, log-view-expanded-log-entry-function.
So one thing we could do is add a VC backend action which returns just
the message text that a human might want to edit.
Probably a backend action called `get-change-comment'.
I think, though, that there might be subtle complexities there. For
example, should there be a FILES argument, or just a REVISION argument?
For Git and Hg it's just REVISION, but we wouldn't want to bake that in.
I think, therefore, that the approach of parsing text out of the
log-view buffer is more future-proof, even though it's complex.
So please see the following WIP patch.
-- >8 --
---
lisp/vc/log-view.el | 48 +++++++++++++++++++++++++++++++--------------
lisp/vc/vc-git.el | 9 +++++++++
lisp/vc/vc-hg.el | 8 ++++++++
3 files changed, 50 insertions(+), 15 deletions(-)
diff --git a/lisp/vc/log-view.el b/lisp/vc/log-view.el
index e9e6602e414..f2bfe642390 100644
--- a/lisp/vc/log-view.el
+++ b/lisp/vc/log-view.el
@@ -520,23 +520,41 @@ log-view-find-revision
log-view-vc-backend))))
+(defun log-view--default-extract-comment-function ()
+ (when (memq log-view-vc-backend '(SCCS RCS CVS SVN))
+ (delete-region (pos-bol) (pos-bol 3)))
+ (goto-char (point-max))
+ (when (eq log-view-vc-backend 'SVN)
+ (delete-region (pos-bol 0) (point)))
+ (buffer-string))
+
+;; We want the possibility of something backend-specific here because
+;; there are all sorts of possibilities for how the comment needs to be
+;; extracted. For example, if the user has customized a variable like
+;; `vc-git-log-switches' then that could change how to parse out the
+;; message in `vc-git-log-view-mode'.
+(defvar log-view-extract-comment-function
+ #'log-view--default-extract-comment-function
+ "Function to return the free text part of a Log View entry.
+`log-view-extract-comment' calls this with no arguments in a
+temporary buffer containing the full text of the Log View entry.
+The default value works for the SCCS, RCS, CVS and SVN backends.")
+
(defun log-view-extract-comment ()
"Parse comment from around the current point in the log."
- (save-excursion
- (let (st en (backend (vc-backend (log-view-current-file))))
- (log-view-end-of-defun)
- (cond ((eq backend 'SVN)
- (forward-line -1)))
- (setq en (point))
- (or (log-view-current-entry nil t)
- (throw 'beginning-of-buffer nil))
- (cond ((memq backend '(SCCS RCS CVS SVN))
- (forward-line 2))
- ((eq backend 'Hg)
- (forward-line 4)
- (re-search-forward "summary: *" nil t)))
- (setq st (point))
- (buffer-substring st en))))
+ (let* ((entry (or (log-view-current-entry)
+ (throw 'beginning-of-buffer nil)))
+ (text (if log-view-expanded-log-entry-function
+ (funcall log-view-expanded-log-entry-function
+ (cadr entry))
+ (save-excursion
+ (goto-char (car entry))
+ (log-view-end-of-defun)
+ (buffer-substring (car entry) (point))))))
+ (with-temp-buffer
+ (insert text)
+ (goto-char (point-min))
+ (funcall log-view-extract-comment-function))))
(declare-function vc-modify-change-comment "vc" (files rev oldcomment))
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 05400523048..0af4e4e4600 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1577,6 +1577,7 @@ log-view-file-re
(defvar log-view-font-lock-keywords)
(defvar log-view-per-file-logs)
(defvar log-view-expanded-log-entry-function)
+(defvar log-view-extract-comment-function)
(define-derived-mode vc-git-log-view-mode log-view-mode "Git-Log-View"
(require 'add-log) ;; We need the faces add-log.
@@ -1592,6 +1593,8 @@ vc-git-log-view-mode
(setq truncate-lines t)
(setq-local log-view-expanded-log-entry-function
'vc-git-expanded-log-entry))
+ (setq-local log-view-extract-comment-function
+ #'vc-git--extract-comment)
(setq-local log-view-font-lock-keywords
(if (not (memq vc-log-view-type '(long log-search with-diff)))
(list (cons (nth 1 vc-git-root-log-format)
@@ -1650,6 +1653,12 @@ vc-git-expanded-log-entry
(forward-line))
(buffer-string))))
+(defun vc-git--extract-comment ()
+ (re-search-forward "^ " nil t)
+ (delete-region (point-min) (point))
+ ;; now deindent
+)
+
(defun vc-git-region-history (file buffer lfrom lto)
"Insert into BUFFER the history of FILE for lines LFROM to LTO.
This requires git 1.8.4 or later, for the \"-L\" option of \"git log\"."
diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index 876d86dc24f..9a9c9c41997 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -425,6 +425,7 @@ log-view-file-re
(defvar log-view-font-lock-keywords)
(defvar log-view-per-file-logs)
(defvar log-view-expanded-log-entry-function)
+(defvar log-view-extract-comment-function)
(define-derived-mode vc-hg-log-view-mode log-view-mode "Hg-Log-View"
(require 'add-log) ;; we need the add-log faces
@@ -440,6 +441,8 @@ vc-hg-log-view-mode
(setq truncate-lines t)
(setq-local log-view-expanded-log-entry-function
'vc-hg-expanded-log-entry))
+ (setq-local log-view-extract-comment-function
+ #'vc-hg--extract-comment)
(setq-local log-view-font-lock-keywords
(if (eq vc-log-view-type 'short)
(list (cons (nth 1 vc-hg-root-log-format)
@@ -541,6 +544,11 @@ vc-hg-expanded-log-entry
(goto-char (point-max))
(buffer-string))))
+(defun vc-hg--extract-comment ()
+ (forward-line 4)
+ (re-search-forward "summary: *" nil t)
+ (buffer-substring (point) (point-max)))
+
(defun vc-hg-revision-table (files)
(let ((default-directory (file-name-directory (car files))))
(with-temp-buffer
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Thu, 10 Oct 2024 02:46:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Tue 13 Jun 2023 at 06:59pm -04, Morgan Smith wrote:
> diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
> index a3469b71386..18d6f1f47dc 100644
> --- a/lisp/vc/vc-git.el
> +++ b/lisp/vc/vc-git.el
> @@ -1384,6 +1384,24 @@ vc-git-clone
> (vc-git--out-ok "clone" remote directory))
> directory)
>
> +(defun vc-git-modify-change-comment (_files rev comment)
> + "Modify the change comments on REV to COMMENT."
> + ;; This is very similar to using to "git commit --fixup=amend"
> + ;; command but it is more precise as it does the rebase matching
> + ;; with the hash instead of the subject line. Also we can't use
> + ;; --fixup non-interactively (it doesn't support -m or -F) so this
> + ;; is much easier.
> + (vc-git-command nil 0 nil "commit"
> + "--allow-empty"
> + "-m" (concat "amend! " rev "\n\n" comment))
> + ;; We should really be able to do this "non-interactively" but we
> + ;; can't so we set GIT_SEQUENCE_EDITOR
> + (let ((process-environment
> + (cons
> + "GIT_SEQUENCE_EDITOR=:"
> + process-environment)))
> + (vc-git-command nil 0 nil "rebase" "--autosquash" "-i" (concat rev "~1"))))
> +
> ;;; HISTORY FUNCTIONS
A few notes here:
- We will need to ensure that the commit is actually empty, not just
allow it to be empty.
We have vc-git--stash-staged-changes we could use, but on my machine
git-commit has --fixup=reword:... which is for amending only the
commit message. So probably use that.
- We will probably want to pass --autostash to git-rebase, too.
- It is probably best to set GIT_SEQUENCE_EDITOR=true so that we're not
relying on the shell -- there is no /bin/:, but there is /bin/true.
This is minor.
> @@ -1576,7 +1594,13 @@ vc-git-expanded-log-entry
> (apply #'vc-git-command t nil nil
> `("log"
> ,revision
> - "-1" "--no-color" ,@(ensure-list vc-git-log-switches)
> + "-1" "--no-color"
> + ;; The same as the default "medium" format but it doesn't
> + ;; put spaces at the beginning of the body. This is so
> + ;; we can grab this as the initial value when calling
> + ;; log-view-modify-change-comment
> + "--pretty=format:commit %H%nAuthor: %an %ae%nDate: %ad%n%n%B"
> + ,@(ensure-list vc-git-log-switches)
> "--"))
> (goto-char (point-min))
> (unless (eobp)
I think it's preferable to use a vc-git--extract-comment as in the patch
I just posted.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Thu, 10 Oct 2024 02:49:01 GMT)
Full text and
rfc822 format available.
Message #50 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Thu 10 Oct 2024 at 10:39am +08, Sean Whitton wrote:
> So one thing we could do is add a VC backend action which returns just
> the message text that a human might want to edit.
> Probably a backend action called `get-change-comment'.
>
> I think, though, that there might be subtle complexities there. For
> example, should there be a FILES argument, or just a REVISION argument?
> For Git and Hg it's just REVISION, but we wouldn't want to bake that in.
>
> I think, therefore, that the approach of parsing text out of the
> log-view buffer is more future-proof, even though it's complex.
Hmm. We could just include all arguments that might be wanted and then
backend functions can ignore them.
I would appreciate opinions comparing adding a new get-change-comment VC
backend function with something like the WIP I posted.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Thu, 10 Oct 2024 06:13:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Date: Thu, 10 Oct 2024 10:45:27 +0800
>
> - It is probably best to set GIT_SEQUENCE_EDITOR=true so that we're not
> relying on the shell -- there is no /bin/:, but there is /bin/true.
Does any of this work on MS-Windows?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Thu, 10 Oct 2024 06:24:01 GMT)
Full text and
rfc822 format available.
Message #56 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Thu 10 Oct 2024 at 09:12am +03, Eli Zaretskii wrote:
>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Date: Thu, 10 Oct 2024 10:45:27 +0800
>>
>> - It is probably best to set GIT_SEQUENCE_EDITOR=true so that we're not
>> relying on the shell -- there is no /bin/:, but there is /bin/true.
>
> Does any of this work on MS-Windows?
I think git on Windows always comes with a POSIX shell, right?
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Thu, 10 Oct 2024 07:40:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: Morgan.J.Smith <at> outlook.com, 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru,
> rpluim <at> gmail.com
> Date: Thu, 10 Oct 2024 14:23:09 +0800
>
> Hello,
>
> On Thu 10 Oct 2024 at 09:12am +03, Eli Zaretskii wrote:
>
> >> From: Sean Whitton <spwhitton <at> spwhitton.name>
> >> Date: Thu, 10 Oct 2024 10:45:27 +0800
> >>
> >> - It is probably best to set GIT_SEQUENCE_EDITOR=true so that we're not
> >> relying on the shell -- there is no /bin/:, but there is /bin/true.
> >
> > Does any of this work on MS-Windows?
>
> I think git on Windows always comes with a POSIX shell, right?
It does, but that doesn't mean it invokes the editor specified by
GIT_SEQUENCE_EDITOR via that shell.
Someone should actually test this to see if it works to use : or
/bin/true there.
Or maybe there's a safer value which will yield the same effect?
E.g., even using just "true", without the "/bin/" part is a step
towards a more portable command, because the user could have a port of
GNU Coreutils, which includes 'true', installed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Thu, 10 Oct 2024 07:47:02 GMT)
Full text and
rfc822 format available.
Message #62 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Thu 10 Oct 2024 at 10:36am +03, Eli Zaretskii wrote:
>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Cc: Morgan.J.Smith <at> outlook.com, 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru,
>> rpluim <at> gmail.com
>> Date: Thu, 10 Oct 2024 14:23:09 +0800
>>
>> Hello,
>>
>> On Thu 10 Oct 2024 at 09:12am +03, Eli Zaretskii wrote:
>>
>> >> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> >> Date: Thu, 10 Oct 2024 10:45:27 +0800
>> >>
>> >> - It is probably best to set GIT_SEQUENCE_EDITOR=true so that we're not
>> >> relying on the shell -- there is no /bin/:, but there is /bin/true.
>> >
>> > Does any of this work on MS-Windows?
>>
>> I think git on Windows always comes with a POSIX shell, right?
>
> It does, but that doesn't mean it invokes the editor specified by
> GIT_SEQUENCE_EDITOR via that shell.
>
> Someone should actually test this to see if it works to use : or
> /bin/true there.
>
> Or maybe there's a safer value which will yield the same effect?
> E.g., even using just "true", without the "/bin/" part is a step
> towards a more portable command, because the user could have a port of
> GNU Coreutils, which includes 'true', installed.
Yeah, I think 'true' is the most portable.
We should test this on MS-Windows later, indeed.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Thu, 17 Oct 2024 13:28:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Thu 10 Oct 2024 at 10:48am +08, Sean Whitton wrote:
> I would appreciate opinions comparing adding a new get-change-comment
> VC backend function with something like the WIP I posted.
I went with adding a backend function. That's now on master. With the
other changes I've made, I believe that all that remains for this bug is
implementing vc-git-modify-change-comment based on our discussion.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Fri, 18 Oct 2024 00:48:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hi! Sorry for the late reply, I've been driving a lot the last week.
On 10/10/2024 05:39, Sean Whitton wrote:
> In Dmitry's patch he takes the approach of calling the
> expanded-log-entry backend function to get the message to edit.
> This is not a real VC backend function -- in fact it's a log-view
> feature, log-view-expanded-log-entry-function.
It is, but it is also basically a backend method, i.e. in vc-git.el:
(setq-local log-view-expanded-log-entry-function
'vc-git-expanded-log-entry))
So if we determined that its output could be used for editing, perhaps
after some massaging (e.g. reindenting and keeping only a subset of the
headers), that could be a minor win -- fewer methods is better in
general. I haven't tried to code it, so there could be pitfalls.
Also note that we have 'rfc822-goto-eoh' which can be used to skip to
the end of the headers. 'log-edit-extract-headers' could be used as
reference for extracting "Summary", even if it doesn't exactly give us
the desired info now.
> So one thing we could do is add a VC backend action which returns just
> the message text that a human might want to edit.
> Probably a backend action called `get-change-comment'.
The new vc-git-get-change-comment seems good in terms of functionality.
I was thinking that the headers such as Author, No-Verify and Sign-Off,
might be good to show as well, but as long as their values are intact
after the edit, that's optional.
> I think, though, that there might be subtle complexities there. For
> example, should there be a FILES argument, or just a REVISION argument?
> For Git and Hg it's just REVISION, but we wouldn't want to bake that in.
Makes sense, e.g. if we end up supporting per-file backends later.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Fri, 18 Oct 2024 04:51:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Fri 18 Oct 2024 at 03:46am +03, Dmitry Gutov wrote:
> Hi! Sorry for the late reply, I've been driving a lot the last week.
Oh, no problem.
> On 10/10/2024 05:39, Sean Whitton wrote:
>> In Dmitry's patch he takes the approach of calling the
>> expanded-log-entry backend function to get the message to edit.
>> This is not a real VC backend function -- in fact it's a log-view
>> feature, log-view-expanded-log-entry-function.
>
> It is, but it is also basically a backend method, i.e. in vc-git.el:
>
> (setq-local log-view-expanded-log-entry-function
> 'vc-git-expanded-log-entry))
>
> So if we determined that its output could be used for editing, perhaps after
> some massaging (e.g. reindenting and keeping only a subset of the headers),
> that could be a minor win -- fewer methods is better in general. I haven't
> tried to code it, so there could be pitfalls.
I thought about it and realised that, for the git case, variables like
vc-git-log-switches and vc-git-shortlog-switches can affect the output,
and could make us misparse it. And ultimately the benefits of avoiding
a new backend method didn't seem to outweigh having parsing code that
could turn out to be fragile.
> Also note that we have 'rfc822-goto-eoh' which can be used to skip to the end
> of the headers. 'log-edit-extract-headers' could be used as reference for
> extracting "Summary", even if it doesn't exactly give us the desired info now.
Those aren't VC-specific, though, they're based on how Log Edit works
for all backends. So although it's fiddly parsing, it can be done the
same way always, at least.
> The new vc-git-get-change-comment seems good in terms of functionality. I was
> thinking that the headers such as Author, No-Verify and Sign-Off, might be
> good to show as well, but as long as their values are intact after the edit,
> that's optional.
I thought that it would be nice to include Author, in particular, as you
might need to amend the value. That would require adding additional
arguments to the modify-change-comment action, though, so I left it for
later if someone wants to implement it.
I'm not sure how No-Verify would interact with the --fixup=reword:
commits we are now planning to use.
Sign-Off would be useful for using the new feature to insert a missing
sign-off.
>> I think, though, that there might be subtle complexities there. For
>> example, should there be a FILES argument, or just a REVISION argument?
>> For Git and Hg it's just REVISION, but we wouldn't want to bake that in.
>
> Makes sense, e.g. if we end up supporting per-file backends later.
Right.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Fri, 18 Oct 2024 05:27:01 GMT)
Full text and
rfc822 format available.
Message #74 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>, Robert Pluim <rpluim <at> gmail.com>,
> Dmitry Gutov <dmitry <at> gutov.dev>
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Date: Thu, 17 Oct 2024 21:27:10 +0800
>
> Hello,
>
> On Thu 10 Oct 2024 at 10:48am +08, Sean Whitton wrote:
>
> > I would appreciate opinions comparing adding a new get-change-comment
> > VC backend function with something like the WIP I posted.
>
> I went with adding a backend function. That's now on master. With the
> other changes I've made, I believe that all that remains for this bug is
> implementing vc-git-modify-change-comment based on our discussion.
Thanks.
The commit log message for the changes doesn't mention the bug number;
please always remember to do that in the future. In this case, the
discussion leading to the change is long and significant, so including
a pointer to it is important for future forensics.
More importantly, why are we installing changes in VC that are
supported by a single backend? This is against the VC design
principles. Let's please install soon implementations for other
relevant backends. I think this should be very simple, but if you
need help for some backends, feel free to ask for it.
Btw, since CVS has an existing command-line option to amend the log
message of a particular commit, I wonder whether the design that goes
through a separate backend function which extracts the comment first
is valid.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Fri, 18 Oct 2024 06:21:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Fri 18 Oct 2024 at 08:26am +03, Eli Zaretskii wrote:
> The commit log message for the changes doesn't mention the bug number;
> please always remember to do that in the future. In this case, the
> discussion leading to the change is long and significant, so including
> a pointer to it is important for future forensics.
In this case, my commits didn't add vc-git-modify-change-comment, and
most of the discussion in this bug has been about that. So, I thought
that it would be best to include the bug number in the later commit
adding vc-git-modify-change-comment, and not in any commits I've made so
far. Would you prefer that it had been included in all of them?
> More importantly, why are we installing changes in VC that are
> supported by a single backend? This is against the VC design
> principles. Let's please install soon implementations for other
> relevant backends. I think this should be very simple, but if you
> need help for some backends, feel free to ask for it.
Actually, in this case, the functionality already exists for SCCS, RCS,
CVS, SVN and Hg in log-view-extract-comment, and I was just adding
support for Git.
> Btw, since CVS has an existing command-line option to amend the log
> message of a particular commit, I wonder whether the design that goes
> through a separate backend function which extracts the comment first
> is valid.
We still need to extract the comment first so that the user has some
text to edit.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Fri, 18 Oct 2024 09:17:01 GMT)
Full text and
rfc822 format available.
Message #80 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, Morgan.J.Smith <at> outlook.com, rpluim <at> gmail.com,
> dmitry <at> gutov.dev
> Date: Fri, 18 Oct 2024 14:20:23 +0800
>
> On Fri 18 Oct 2024 at 08:26am +03, Eli Zaretskii wrote:
>
> > The commit log message for the changes doesn't mention the bug number;
> > please always remember to do that in the future. In this case, the
> > discussion leading to the change is long and significant, so including
> > a pointer to it is important for future forensics.
>
> In this case, my commits didn't add vc-git-modify-change-comment, and
> most of the discussion in this bug has been about that. So, I thought
> that it would be best to include the bug number in the later commit
> adding vc-git-modify-change-comment, and not in any commits I've made so
> far. Would you prefer that it had been included in all of them?
Definitely. It is otherwise very hard to realize that the discussion
of these changes happened in this particular bug.
> > More importantly, why are we installing changes in VC that are
> > supported by a single backend? This is against the VC design
> > principles. Let's please install soon implementations for other
> > relevant backends. I think this should be very simple, but if you
> > need help for some backends, feel free to ask for it.
>
> Actually, in this case, the functionality already exists for SCCS, RCS,
> CVS, SVN and Hg in log-view-extract-comment, and I was just adding
> support for Git.
That's definitely not what the comments and the implementation convey.
And I then understand even less why the backend action was added for
Git in the first place.
> > Btw, since CVS has an existing command-line option to amend the log
> > message of a particular commit, I wonder whether the design that goes
> > through a separate backend function which extracts the comment first
> > is valid.
>
> We still need to extract the comment first so that the user has some
> text to edit.
Are we envisioning the user invoking this from anything other than a
log buffer? Why would someone want to do that?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Fri, 18 Oct 2024 09:28:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 64055 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello,
This patch is the rest of this feature. Eli, would you kindly test on
MS-Windows, please? Just try to edit the message for an unpushed git
commit and add some Unicode, and see if it works correctly.
--
Sean Whitton
[0001-Support-modifying-VC-change-comments-for-Git.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Fri, 18 Oct 2024 09:31:02 GMT)
Full text and
rfc822 format available.
Message #86 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Fri 18 Oct 2024 at 12:14pm +03, Eli Zaretskii wrote:
> Definitely. It is otherwise very hard to realize that the discussion
> of these changes happened in this particular bug.
Right, okay.
> That's definitely not what the comments and the implementation convey.
> And I then understand even less why the backend action was added for
> Git in the first place.
Hmm, I see what you mean. I should have included more detail in the
commit message and/or comments.
>> > Btw, since CVS has an existing command-line option to amend the log
>> > message of a particular commit, I wonder whether the design that goes
>> > through a separate backend function which extracts the comment first
>> > is valid.
>>
>> We still need to extract the comment first so that the user has some
>> text to edit.
>
> Are we envisioning the user invoking this from anything other than a
> log buffer? Why would someone want to do that?
I can imagine a few cases where it might be useful, but I don't think
that's the main point.
The main point is that parsing text out of the log view buffer is much
more error-prone. For example, with a non-distributed VCS like CVS,
what if someone else has edited the comment in the meantime? It makes
sense to fetch the latest known version instead of relying on buffer
text.
So, I hope that we will eventually completely replace
log-view-extract-comment.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Fri, 18 Oct 2024 12:19:02 GMT)
Full text and
rfc822 format available.
Message #89 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, Morgan.J.Smith <at> outlook.com, rpluim <at> gmail.com,
> dmitry <at> gutov.dev
> Date: Fri, 18 Oct 2024 17:30:16 +0800
>
> The main point is that parsing text out of the log view buffer is much
> more error-prone. For example, with a non-distributed VCS like CVS,
> what if someone else has edited the comment in the meantime? It makes
> sense to fetch the latest known version instead of relying on buffer
> text.
Now I'm confused: didn't you just tell me that CVS already had this
implemented, without ever needing the new backend method? How do we
solve this problem in the CVS case without risking the errors? What
you say above seems to tell that we do need an implementation of this
method for CVS as well, no? Or what am I missing?
> So, I hope that we will eventually completely replace
> log-view-extract-comment.
So you do agree with me that we should have this implemented for other
backends, and the sooner the better?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sat, 19 Oct 2024 10:30:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: Dmitry Gutov <dgutov <at> yandex.ru>, Morgan Smith <Morgan.J.Smith <at> outlook.com>
> Date: Fri, 18 Oct 2024 17:26:28 +0800
>
> This patch is the rest of this feature. Eli, would you kindly test on
> MS-Windows, please? Just try to edit the message for an unpushed git
> commit and add some Unicode, and see if it works correctly.
I'm supposed to type 'e' in the log buffer, edit the comment, then
type "C-c C-c"? Or should I do something else?
When I do the above, I get an error message:
vc-do-command: Failed (status 128): git --no-pager commit --only --allow-empty -F c:/Users/EliZ/AppData/Local/Temp/git-msgWaNflu
and the *vc* buffer says:
fatal: No paths with --include/--only does not make sense.
It's possible my Git is very old, but shouldn't this command work with
old versions as well?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 00:18:01 GMT)
Full text and
rfc822 format available.
Message #95 received at 64055 <at> debbugs.gnu.org (full text, mbox):
On 18/10/2024 07:50, Sean Whitton wrote:
>> So if we determined that its output could be used for editing, perhaps after
>> some massaging (e.g. reindenting and keeping only a subset of the headers),
>> that could be a minor win -- fewer methods is better in general. I haven't
>> tried to code it, so there could be pitfalls.
> I thought about it and realised that, for the git case, variables like
> vc-git-log-switches and vc-git-shortlog-switches can affect the output,
> and could make us misparse it. And ultimately the benefits of avoiding
> a new backend method didn't seem to outweigh having parsing code that
> could turn out to be fragile.
IIUC vc-git-expanded-log-entry is only affected by the former var. We
could bind it to nil (that could even be done generically, by looking
for all vars called vc-BACKEND-log-switches).
But I agree fragility could be a problem. It could also be a win to have
one of the methods to be implemented in terms of the other.
>> The new vc-git-get-change-comment seems good in terms of functionality. I was
>> thinking that the headers such as Author, No-Verify and Sign-Off, might be
>> good to show as well, but as long as their values are intact after the edit,
>> that's optional.
> I thought that it would be nice to include Author, in particular, as you
> might need to amend the value. That would require adding additional
> arguments to the modify-change-comment action, though, so I left it for
> later if someone wants to implement it.
I think you're parsing out all the headers from the COMMENT argument in
the posted patch, so this paragraph seems to be moot. Like you say in
the patch though, it seems changing Author is not supported.
To do that, apparently we'd either need to rebase with a pause (with
'git amend' in the middle), or use 'git rebase --exec' like in
https://stackoverflow.com/a/79068024/615245. The latter seems like it
would apply to all commits in the range, so it might require the same
dance as 'amend'.
(I can comment on the patch itself tomorrow.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 00:57:01 GMT)
Full text and
rfc822 format available.
Message #98 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Fri 18 Oct 2024 at 03:18pm +03, Eli Zaretskii wrote:
>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Cc: 64055 <at> debbugs.gnu.org, Morgan.J.Smith <at> outlook.com, rpluim <at> gmail.com,
>> dmitry <at> gutov.dev
>> Date: Fri, 18 Oct 2024 17:30:16 +0800
>>
>> The main point is that parsing text out of the log view buffer is much
>> more error-prone. For example, with a non-distributed VCS like CVS,
>> what if someone else has edited the comment in the meantime? It makes
>> sense to fetch the latest known version instead of relying on buffer
>> text.
>
> Now I'm confused: didn't you just tell me that CVS already had this
> implemented, without ever needing the new backend method? How do we
> solve this problem in the CVS case without risking the errors? What
> you say above seems to tell that we do need an implementation of this
> method for CVS as well, no? Or what am I missing?
>
>> So, I hope that we will eventually completely replace
>> log-view-extract-comment.
>
> So you do agree with me that we should have this implemented for other
> backends, and the sooner the better?
The VC design principle here is that commands should be implemented for
more than one version control system, in part because that way we ensure
they really are general version control user operations, and not
something parochial to the way a particular VCS sees the world.
In the context of this bug, the relevant command for this design
principle is log-view-modify-change-comment. Previously, that command
worked only for src, rcs, sccs, cvs and svn, and partly for Hg. Soon it
will also work for git. So, the design principle is respected.
On the other hand, the design principle doesn't apply directly to the
new get-change-comment backend action, because src, rcs, sccs, cvs and
svn do not *require* an implementation of get-change-comment in order
for the log-view-modify-change-comment command to work, so far as I
understand it -- otherwise the feature would not have been implemented
the way it is, or we'd have bug reports.
Git, I think, does require an implementation of get-change-comment, or
at least, I concluded that it's the most natural way to implement
log-view-modify-change-comment for git.
In a similar vein, non-distributed VCS require a receive-file backend
action, but it would not make sense to write a function
vc-git-receive-file. The VC design principle does not apply directly to
backend actions. For many backend actions, including
get-change-comment, only certain kinds of VCS will require an
implementation, in order to support the high-level VC user commands.
Nevertheless, although src, rcs, sccs, cvs and svn do not *require*
get-change-comment, I think it would be preferable if they did have an
implementation, instead of using log-view-extract-comment, for the
reasons given previously. But we are talking about changing working
code. We should not rush to replace log-view-extract-comment. We can
do so incrementally and gradually, and it's low priority.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 01:00:01 GMT)
Full text and
rfc822 format available.
Message #101 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sun 20 Oct 2024 at 01:16am +01, Dmitry Gutov wrote:
>> I thought that it would be nice to include Author, in particular, as you
>> might need to amend the value. That would require adding additional
>> arguments to the modify-change-comment action, though, so I left it for
>> later if someone wants to implement it.
>
> I think you're parsing out all the headers from the COMMENT argument in the
> posted patch, so this paragraph seems to be moot.
Yeah, I realised that because we are invoking git-commit(1), it's easy
to support most of them.
> Like you say in the patch though, it seems changing Author is not
> supported.
>
> To do that, apparently we'd either need to rebase with a pause (with 'git
> amend' in the middle), or use 'git rebase --exec' like in
> https://stackoverflow.com/a/79068024/615245. The latter seems like it would
> apply to all commits in the range, so it might require the same dance as
> 'amend'.
Yeah. We can consider it as a possible future enhancement.
I'm also concerned about it making the command slower -- it's already
not that fast.
> (I can comment on the patch itself tomorrow.)
Cool, thanks.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 05:02:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, Morgan.J.Smith <at> outlook.com, rpluim <at> gmail.com,
> dmitry <at> gutov.dev
> Date: Sun, 20 Oct 2024 08:56:12 +0800
>
> On the other hand, the design principle doesn't apply directly to the
> new get-change-comment backend action, because src, rcs, sccs, cvs and
> svn do not *require* an implementation of get-change-comment in order
> for the log-view-modify-change-comment command to work, so far as I
> understand it -- otherwise the feature would not have been implemented
> the way it is, or we'd have bug reports.
>
> Git, I think, does require an implementation of get-change-comment, or
> at least, I concluded that it's the most natural way to implement
> log-view-modify-change-comment for git.
It is this difference between Git and the rest that I don't yet see,
and you didn't explain it, just stated its existence. Why is the
method necessary for Git, but not for other VCSes? they all produce
similar displays of log messages, complete with Author, Date, etc.,
and so the difficulties of extracting just the change log message
should affect all of them, no?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 05:20:02 GMT)
Full text and
rfc822 format available.
Message #107 received at 64055 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello,
On Sat 19 Oct 2024 at 01:28pm +03, Eli Zaretskii wrote:
>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Cc: Dmitry Gutov <dgutov <at> yandex.ru>, Morgan Smith <Morgan.J.Smith <at> outlook.com>
>> Date: Fri, 18 Oct 2024 17:26:28 +0800
>>
>> This patch is the rest of this feature. Eli, would you kindly test on
>> MS-Windows, please? Just try to edit the message for an unpushed git
>> commit and add some Unicode, and see if it works correctly.
>
> I'm supposed to type 'e' in the log buffer, edit the comment, then
> type "C-c C-c"?
Yeah, that's right.
> When I do the above, I get an error message:
>
> vc-do-command: Failed (status 128): git --no-pager commit --only
> --allow-empty -F c:/Users/EliZ/AppData/Local/Temp/git-msgWaNflu
>
> and the *vc* buffer says:
>
> fatal: No paths with --include/--only does not make sense.
>
> It's possible my Git is very old, but shouldn't this command work with
> old versions as well?
I've looked into it and what's required is Git 2.11.1 from early 2017.
I think we can support older by stashing and unstashing. So please try
the attached patch, which does that.
--
Sean Whitton
[v2-0001-Support-modifying-VC-change-comments-for-Git.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 05:31:02 GMT)
Full text and
rfc822 format available.
Message #110 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sun 20 Oct 2024 at 07:58am +03, Eli Zaretskii wrote:
> It is this difference between Git and the rest that I don't yet see,
> and you didn't explain it, just stated its existence. Why is the
> method necessary for Git, but not for other VCSes? they all produce
> similar displays of log messages, complete with Author, Date, etc.,
> and so the difficulties of extracting just the change log message
> should affect all of them, no?
Ah, right.
The difference is that with Git you can use variables like
vc-git-log-switches and vc-git-shortlog-switches to radically change the
log output. We can't realistically deal with all the possibilities from
Lisp.
Whereas, so far as I understand, such customisation of the output is not
available with the older VCS.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 06:10:01 GMT)
Full text and
rfc822 format available.
Message #113 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, Morgan.J.Smith <at> outlook.com, rpluim <at> gmail.com,
> dmitry <at> gutov.dev
> Date: Sun, 20 Oct 2024 13:29:59 +0800
>
> Hello,
>
> On Sun 20 Oct 2024 at 07:58am +03, Eli Zaretskii wrote:
>
> > It is this difference between Git and the rest that I don't yet see,
> > and you didn't explain it, just stated its existence. Why is the
> > method necessary for Git, but not for other VCSes? they all produce
> > similar displays of log messages, complete with Author, Date, etc.,
> > and so the difficulties of extracting just the change log message
> > should affect all of them, no?
>
> Ah, right.
>
> The difference is that with Git you can use variables like
> vc-git-log-switches and vc-git-shortlog-switches to radically change the
> log output. We can't realistically deal with all the possibilities from
> Lisp.
>
> Whereas, so far as I understand, such customisation of the output is not
> available with the older VCS.
I see vc-hg-log-switches and vc-bzr-log-switches, so I don't think I
understand why you say Git is special here. What did I miss?
In any case, the reason for having a backend method should be
explained in the comments.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 07:20:01 GMT)
Full text and
rfc822 format available.
Message #116 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sun 20 Oct 2024 at 09:09am +03, Eli Zaretskii wrote:
> I see vc-hg-log-switches and vc-bzr-log-switches, so I don't think I
> understand why you say Git is special here. What did I miss?
By "older VCS" I did not mean to include Hg and bzr, which are special
in the same way as Git. And indeed, neither vc-bzr or vc-hg have
log-view-modify-change-comment support. It would be good to add it.
> In any case, the reason for having a backend method should be
> explained in the comments.
I've added a FIXME pointing to this bug. Is there somewhere else you
have in mind?
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 08:21:01 GMT)
Full text and
rfc822 format available.
Message #119 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, Morgan.J.Smith <at> outlook.com, rpluim <at> gmail.com,
> dmitry <at> gutov.dev
> Date: Sun, 20 Oct 2024 15:18:38 +0800
>
> Hello,
>
> On Sun 20 Oct 2024 at 09:09am +03, Eli Zaretskii wrote:
>
> > I see vc-hg-log-switches and vc-bzr-log-switches, so I don't think I
> > understand why you say Git is special here. What did I miss?
>
> By "older VCS" I did not mean to include Hg and bzr, which are special
> in the same way as Git. And indeed, neither vc-bzr or vc-hg have
> log-view-modify-change-comment support. It would be good to add it.
I consider this change incomplete until we add that support at least
for Hg.
> > In any case, the reason for having a backend method should be
> > explained in the comments.
>
> I've added a FIXME pointing to this bug. Is there somewhere else you
> have in mind?
I don't see it; did you forget to push? In any case, I prefer an
explicit explanation, including the reference to the vc-*-log-switches
variables, not just a reference to a bug number.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 08:34:02 GMT)
Full text and
rfc822 format available.
Message #122 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
> Date: Sun, 20 Oct 2024 13:19:09 +0800
>
> > When I do the above, I get an error message:
> >
> > vc-do-command: Failed (status 128): git --no-pager commit --only
> > --allow-empty -F c:/Users/EliZ/AppData/Local/Temp/git-msgWaNflu
> >
> > and the *vc* buffer says:
> >
> > fatal: No paths with --include/--only does not make sense.
> >
> > It's possible my Git is very old, but shouldn't this command work with
> > old versions as well?
>
> I've looked into it and what's required is Git 2.11.1 from early 2017.
So someone will have to test the patch on Windows with a new en ough
Git version.
> I think we can support older by stashing and unstashing. So please try
> the attached patch, which does that.
This seems to work without any errors, and the *vc* buffer says
Successfully rebased and updated refs/heads/master.
But the log message is not updated, neither in the *vc-change-log*
buffer nor if I manually invoke "git log" from the shell prompt. It
sounds like the commit message was not amended. "git stash list" also
shows no relevant stashes.
Let me know how can I help you debug this.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 08:44:01 GMT)
Full text and
rfc822 format available.
Message #125 received at 64055 <at> debbugs.gnu.org (full text, mbox):
retitle 64055 31.0.50; log-view-modify-change-comment support for Git and Hg
thanks
On Sun 20 Oct 2024 at 11:20am +03, Eli Zaretskii wrote:
> I consider this change incomplete until we add that support at least
> for Hg.
Fair enough. I'm retitling the bug. And after I install the change to
vc-git.el, I won't close this bug.
>> > In any case, the reason for having a backend method should be
>> > explained in the comments.
>>
>> I've added a FIXME pointing to this bug. Is there somewhere else you
>> have in mind?
>
> I don't see it; did you forget to push?
I did push it.
> In any case, I prefer an explicit explanation, including the reference
> to the vc-*-log-switches variables, not just a reference to a bug
> number.
Added a second FIXME regarding modern VCS.
--
Sean Whitton
Changed bug title to '31.0.50; log-view-modify-change-comment support for Git and Hg' from '[WIP Patch] Enable editing commit messages - vc-git-modify-change-comment'
Request was from
Sean Whitton <spwhitton <at> spwhitton.name>
to
control <at> debbugs.gnu.org.
(Sun, 20 Oct 2024 08:44:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 08:58:01 GMT)
Full text and
rfc822 format available.
Message #130 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, Morgan.J.Smith <at> outlook.com, rpluim <at> gmail.com,
> dmitry <at> gutov.dev, control <at> debbugs.gnu.org
> Date: Sun, 20 Oct 2024 16:42:54 +0800
>
> > In any case, I prefer an explicit explanation, including the reference
> > to the vc-*-log-switches variables, not just a reference to a bug
> > number.
>
> Added a second FIXME regarding modern VCS.
Thanks, LGTM.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 09:01:02 GMT)
Full text and
rfc822 format available.
Message #133 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sun 20 Oct 2024 at 11:32am +03, Eli Zaretskii wrote:
> So someone will have to test the patch on Windows with a new en ough
> Git version.
I don't think so -- the code to support Windows is independent of the
code to support older Git. So, my tests here on GNU/Linux with newer
Git, together with yours on Windows with older Git, are sufficient.
>> I think we can support older by stashing and unstashing. So please try
>> the attached patch, which does that.
>
> This seems to work without any errors, and the *vc* buffer says
>
> Successfully rebased and updated refs/heads/master.
>
> But the log message is not updated, neither in the *vc-change-log*
> buffer nor if I manually invoke "git log" from the shell prompt. It
> sounds like the commit message was not amended. "git stash list" also
> shows no relevant stashes.
>
> Let me know how can I help you debug this.
Thank you for testing it.
Let's try commenting out everything except the 'git commit', as done in
the following diff on top of my patch. Please test this without
anything staged. Does it create a new commit at the tip of your branch?
The contents of the commit should be a special first line, followed by
your amended commit message.
If not: I've also commented out the deletion of the temporary file, and
sent its name to *Messages*. If you look in that temporary file, do you
find your amended commit message?
--8<---------------cut here---------------start------------->8---
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 59d5386bc72..d2ada63f71e 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -2042,8 +2042,8 @@ vc-git-modify-change-comment
;; % git commit --allow-empty -m...
;; % git rebase --autosquash -i REV~1
;; % git stash pop
- (unless nothing-staged
- (vc-git-command nil 0 nil "stash" "push"))
+ ;; (unless nothing-staged
+ ;; (vc-git-command nil 0 nil "stash" "push"))
(unwind-protect
(progn
(when (cl-intersection '("--author" "--date") args
@@ -2064,12 +2064,15 @@ vc-git-modify-change-comment
(list "-m" message))
args)))
(when (and msg-file (file-exists-p msg-file))
- (delete-file msg-file)))
- (with-environment-variables (("GIT_SEQUENCE_EDITOR" "true"))
- (vc-git-command nil 0 nil "rebase" "--autosquash" "-i"
- (format "%s~1" rev)))
- (unless nothing-staged
- (vc-git-command nil 0 nil "stash" "pop" "--index"))))
+ ;; (delete-file msg-file)
+ ))
+ ;; (with-environment-variables (("GIT_SEQUENCE_EDITOR" "true"))
+ ;; (vc-git-command nil 0 nil "rebase" "--autosquash" "-i"
+ ;; (format "%s~1" rev)))
+ ;; (unless nothing-staged
+ ;; (vc-git-command nil 0 nil "stash" "pop" "--index"))
+ (message "temporary file is: %s" msg-file)
+ ))
(defvar vc-git-extra-menu-map
(let ((map (make-sparse-keymap)))
--8<---------------cut here---------------end--------------->8---
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 09:21:02 GMT)
Full text and
rfc822 format available.
Message #136 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
> Date: Sun, 20 Oct 2024 16:59:32 +0800
>
> Let's try commenting out everything except the 'git commit', as done in
> the following diff on top of my patch. Please test this without
> anything staged. Does it create a new commit at the tip of your branch?
Yes. But only "git log" shows that additional commit, "C-x v l"
doesn't update the log.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 09:26:02 GMT)
Full text and
rfc822 format available.
Message #139 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sun 20 Oct 2024 at 12:19pm +03, Eli Zaretskii wrote:
>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
>> Date: Sun, 20 Oct 2024 16:59:32 +0800
>>
>> Let's try commenting out everything except the 'git commit', as done in
>> the following diff on top of my patch. Please test this without
>> anything staged. Does it create a new commit at the tip of your branch?
>
> Yes. But only "git log" shows that additional commit, "C-x v l"
> doesn't update the log.
If you press 'g' in the 'C-x v l' buffer, does it appear then?
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 09:48:01 GMT)
Full text and
rfc822 format available.
Message #142 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sun 20 Oct 2024 at 05:25pm +08, Sean Whitton wrote:
> Hello,
>
> On Sun 20 Oct 2024 at 12:19pm +03, Eli Zaretskii wrote:
>
>>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>>> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
>>> Date: Sun, 20 Oct 2024 16:59:32 +0800
>>>
>>> Let's try commenting out everything except the 'git commit', as done in
>>> the following diff on top of my patch. Please test this without
>>> anything staged. Does it create a new commit at the tip of your branch?
>>
>> Yes. But only "git log" shows that additional commit, "C-x v l"
>> doesn't update the log.
>
> If you press 'g' in the 'C-x v l' buffer, does it appear then?
Oh, wait, it wouldn't. Does it appear in 'C-x v L'?
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 11:20:02 GMT)
Full text and
rfc822 format available.
Message #145 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
> Date: Sun, 20 Oct 2024 17:25:10 +0800
>
> Hello,
>
> On Sun 20 Oct 2024 at 12:19pm +03, Eli Zaretskii wrote:
>
> >> From: Sean Whitton <spwhitton <at> spwhitton.name>
> >> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
> >> Date: Sun, 20 Oct 2024 16:59:32 +0800
> >>
> >> Let's try commenting out everything except the 'git commit', as done in
> >> the following diff on top of my patch. Please test this without
> >> anything staged. Does it create a new commit at the tip of your branch?
> >
> > Yes. But only "git log" shows that additional commit, "C-x v l"
> > doesn't update the log.
>
> If you press 'g' in the 'C-x v l' buffer, does it appear then?
No. Not even if I restart Emacs.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 11:25:01 GMT)
Full text and
rfc822 format available.
Message #148 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
> Date: Sun, 20 Oct 2024 17:46:53 +0800
>
> >> Yes. But only "git log" shows that additional commit, "C-x v l"
> >> doesn't update the log.
> >
> > If you press 'g' in the 'C-x v l' buffer, does it appear then?
>
> Oh, wait, it wouldn't. Does it appear in 'C-x v L'?
Yes.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 13:12:01 GMT)
Full text and
rfc822 format available.
Message #151 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sun 20 Oct 2024 at 02:24pm +03, Eli Zaretskii wrote:
>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
>> Date: Sun, 20 Oct 2024 17:46:53 +0800
>>
>> >> Yes. But only "git log" shows that additional commit, "C-x v l"
>> >> doesn't update the log.
>> >
>> > If you press 'g' in the 'C-x v l' buffer, does it appear then?
>>
>> Oh, wait, it wouldn't. Does it appear in 'C-x v L'?
>
> Yes.
Okay. Does the message look like this:
amend! <sha1 of the commit whose message you edited>
<your edited message>
If so, then we know that the invocation of 'git commit' is working, and
the problem is somewhere else -- probably the invocation of 'git rebase'.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 13:54:02 GMT)
Full text and
rfc822 format available.
Message #154 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
> Date: Sun, 20 Oct 2024 21:11:00 +0800
>
> >> Oh, wait, it wouldn't. Does it appear in 'C-x v L'?
> >
> > Yes.
>
> Okay. Does the message look like this:
>
> amend! <sha1 of the commit whose message you edited>
>
> <your edited message>
Yes.
> If so, then we know that the invocation of 'git commit' is working, and
> the problem is somewhere else -- probably the invocation of 'git rebase'.
With you, so far.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 20 Oct 2024 23:56:02 GMT)
Full text and
rfc822 format available.
Message #157 received at 64055 <at> debbugs.gnu.org (full text, mbox):
On 20/10/2024 08:19, Sean Whitton wrote:
>> When I do the above, I get an error message:
>>
>> vc-do-command: Failed (status 128): git --no-pager commit --only
>> --allow-empty -F c:/Users/EliZ/AppData/Local/Temp/git-msgWaNflu
>>
>> and the*vc* buffer says:
>>
>> fatal: No paths with --include/--only does not make sense.
>>
>> It's possible my Git is very old, but shouldn't this command work with
>> old versions as well?
> I've looked into it and what's required is Git 2.11.1 from early 2017.
>
> I think we can support older by stashing and unstashing. So please try
> the attached patch, which does that.
Just to note, looking at https://repology.org/project/git/versions,
support for Git 2.11 seems ubiquitous enough, even the next-to-last
Trisquel release (based on Ubuntu 2020) has 2.25. And the very newest
versions are available in Git for Windows.
And since this is an optional feature, we might as well limit ourselves
to certain newer Git versions.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 00:03:02 GMT)
Full text and
rfc822 format available.
Message #160 received at 64055 <at> debbugs.gnu.org (full text, mbox):
On 20/10/2024 16:52, Eli Zaretskii wrote:
>> From: Sean Whitton<spwhitton <at> spwhitton.name>
>> Cc:64055 <at> debbugs.gnu.org,dgutov <at> yandex.ru,Morgan.J.Smith <at> outlook.com
>> Date: Sun, 20 Oct 2024 21:11:00 +0800
>>
>>>> Oh, wait, it wouldn't. Does it appear in 'C-x v L'?
>>> Yes.
>> Okay. Does the message look like this:
>>
>> amend! <sha1 of the commit whose message you edited>
>>
>> <your edited message>
> Yes.
>
>> If so, then we know that the invocation of 'git commit' is working, and
>> the problem is somewhere else -- probably the invocation of 'git rebase'.
> With you, so far.
Eli, it seems you've installed the patch from this bug on master by
accident, in commit 75fa0cc1ae2.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 00:11:02 GMT)
Full text and
rfc822 format available.
Message #163 received at 64055 <at> debbugs.gnu.org (full text, mbox):
On 20/10/2024 08:19, Sean Whitton wrote:
> +*** New user option 'vc-git-allow-rewriting-history'.
> +Many Git commands can change your copy of published change history
> +without warning. If VC commands detect that this could happen, they
> +will stop. You can customize this variable to permit rewriting history
> +even though Emacs thinks it is dangerous.
Curious: do we consider Git to be different from others? Or would we
have corresponding options for Hg, Bzr, (maybe) Svn?
If we think rewriting history dangerous for all, we could mark the
command as 'disabled' instead.
Otherwise, seems to work well, even though, as you said, a bit slower
than one might naively expect.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 01:47:02 GMT)
Full text and
rfc822 format available.
Message #166 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Mon 21 Oct 2024 at 01:01am +01, Dmitry Gutov wrote:
> Eli, it seems you've installed the patch from this bug on master by accident,
> in commit 75fa0cc1ae2.
The debugging version, it would seem. I've gone ahead and reverted it.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 01:53:01 GMT)
Full text and
rfc822 format available.
Message #169 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sun 20 Oct 2024 at 04:52pm +03, Eli Zaretskii wrote:
>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
>> Date: Sun, 20 Oct 2024 21:11:00 +0800
>>
>> >> Oh, wait, it wouldn't. Does it appear in 'C-x v L'?
>> >
>> > Yes.
>>
>> Okay. Does the message look like this:
>>
>> amend! <sha1 of the commit whose message you edited>
>>
>> <your edited message>
>
> Yes.
>
>> If so, then we know that the invocation of 'git commit' is working, and
>> the problem is somewhere else -- probably the invocation of 'git rebase'.
>
> With you, so far.
I think the next thing to do is try the rebase from the command line.
So, please use the debugging version of my patch to create the special
"amend!" commit again, or use the one from yesterday if you still have
it, or you could manually create it by passing --allow-empty to 'git
commit'.
Then, at the command line, use
git rebase --autosquash -i REV~1
where REV is the sha1 of the commit whose message you intend to edit.
This should open an editor with a rebase plan in which you see
- the commit whose message you wanted to change
- followed by the special amend! commit
- followed by any other commits following the one whose message you wanted to change
i.e. the amend! commit has been pulled down in history to just after the
commit whose message you want to cahnge.
Just save and exit without making any changes to the plan.
After that, the whole process should be complete: you should have the
same commits as previously except the message should have been updated,
and sha1 hashes will have changed.
Let me know if this all works.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 02:03:02 GMT)
Full text and
rfc822 format available.
Message #172 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Mon 21 Oct 2024 at 01:09am +01, Dmitry Gutov wrote:
> On 20/10/2024 08:19, Sean Whitton wrote:
>> +*** New user option 'vc-git-allow-rewriting-history'.
>> +Many Git commands can change your copy of published change history
>> +without warning. If VC commands detect that this could happen, they
>> +will stop. You can customize this variable to permit rewriting history
>> +even though Emacs thinks it is dangerous.
>
> Curious: do we consider Git to be different from others? Or would we have
> corresponding options for Hg, Bzr, (maybe) Svn?
>
> If we think rewriting history dangerous for all, we could mark the command as
> 'disabled' instead.
I think that for some VCS editing the commit message is not a form of
rewriting history in any sense. And we can hope for a future VCS that
keeps change histories for commit messages, so that we might easily
correct our mistakes. So I don't believe it would make sense to just
disable log-view-modify-change-comment.
We might, though, want to consider a more general
vc-allow-rewriting-history instead of vc-git-allow-rewriting-history,
and use it wherever its applicable -- it will be VCS-dependent where
exactly it applies.
I'm cautious about doing that without more information, because
"rewriting history" is, in my mind, a Git-specific piece of terminology
to begin with.
I think it is probably okay to keep vc-git-allow-rewriting-history even
if we decide to deprecate it in favour of a more general variable at
some point.
> Otherwise, seems to work well, even though, as you said, a bit slower than one
> might naively expect.
Ah, thank you for testing.
I'm not whether the slowness is coming from the safety check in
vc-git--assert-allowed-rewrite or from the rebase command, or both. I
do know that rebases don't tend to be fast.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 02:11:02 GMT)
Full text and
rfc822 format available.
Message #175 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Mon 21 Oct 2024 at 12:55am +01, Dmitry Gutov wrote:
> Just to note, looking at https://repology.org/project/git/versions, support
> for Git 2.11 seems ubiquitous enough, even the next-to-last Trisquel release
> (based on Ubuntu 2020) has 2.25. And the very newest versions are available in
> Git for Windows.
>
> And since this is an optional feature, we might as well limit ourselves to
> certain newer Git versions.
AFAICT it's straightforward to support the older git -- basically
replace the --autostash argument to 'git rebase' with a manual stash and
unstash (not because --autostash is unsupported, but because we need the
stash to occur earlier).
If the Windows testing that Eli and I are doing reaches a conclusion
that there are other incompatibilities caused by the older Git, we may
have to abandon trying to support <<2.11.1, so thank you for looking
into the availability.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 05:28:02 GMT)
Full text and
rfc822 format available.
Message #178 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> Date: Mon, 21 Oct 2024 01:01:25 +0100
> Cc: Morgan.J.Smith <at> outlook.com, 64055 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
>
> >> amend! <sha1 of the commit whose message you edited>
> >>
> >> <your edited message>
> > Yes.
> >
> >> If so, then we know that the invocation of 'git commit' is working, and
> >> the problem is somewhere else -- probably the invocation of 'git rebase'.
> > With you, so far.
>
> Eli, it seems you've installed the patch from this bug on master by
> accident, in commit 75fa0cc1ae2.
Oops, thanks, fixed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 05:43:01 GMT)
Full text and
rfc822 format available.
Message #181 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, Morgan.J.Smith <at> outlook.com,
> 64055 <at> debbugs.gnu.org
> Date: Mon, 21 Oct 2024 10:01:28 +0800
>
> > On 20/10/2024 08:19, Sean Whitton wrote:
> >> +*** New user option 'vc-git-allow-rewriting-history'.
> >> +Many Git commands can change your copy of published change history
> >> +without warning. If VC commands detect that this could happen, they
> >> +will stop. You can customize this variable to permit rewriting history
> >> +even though Emacs thinks it is dangerous.
> >
> > Curious: do we consider Git to be different from others? Or would we have
> > corresponding options for Hg, Bzr, (maybe) Svn?
> >
> > If we think rewriting history dangerous for all, we could mark the command as
> > 'disabled' instead.
>
> I think that for some VCS editing the commit message is not a form of
> rewriting history in any sense. And we can hope for a future VCS that
> keeps change histories for commit messages, so that we might easily
> correct our mistakes. So I don't believe it would make sense to just
> disable log-view-modify-change-comment.
>
> We might, though, want to consider a more general
> vc-allow-rewriting-history instead of vc-git-allow-rewriting-history,
> and use it wherever its applicable -- it will be VCS-dependent where
> exactly it applies.
>
> I'm cautious about doing that without more information, because
> "rewriting history" is, in my mind, a Git-specific piece of terminology
> to begin with.
Which aspects of "rewriting history" you consider Git-specific? I
think any dVCS has the same issues with that, and supports similar
workflows, so the concept is relevant to all of them.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 05:48:02 GMT)
Full text and
rfc822 format available.
Message #184 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Mon 21 Oct 2024 at 08:39am +03, Eli Zaretskii wrote:
> Which aspects of "rewriting history" you consider Git-specific? I
> think any dVCS has the same issues with that, and supports similar
> workflows, so the concept is relevant to all of them.
Mainly the terminology.
If others think it is fine to go ahead and call the defcustom
vc-allow-rewriting-history, I am happy to do so.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 05:51:01 GMT)
Full text and
rfc822 format available.
Message #187 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com, 64055 <at> debbugs.gnu.org
> Date: Mon, 21 Oct 2024 13:46:39 +0800
>
> Hello,
>
> On Mon 21 Oct 2024 at 08:39am +03, Eli Zaretskii wrote:
>
> > Which aspects of "rewriting history" you consider Git-specific? I
> > think any dVCS has the same issues with that, and supports similar
> > workflows, so the concept is relevant to all of them.
>
> Mainly the terminology.
I think both rebasing and non-FF pushes can happen with all dVCSes, so
the issue is indeed more general than only with Git.
> If others think it is fine to go ahead and call the defcustom
> vc-allow-rewriting-history, I am happy to do so.
I think we should indeed rename it, yes.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 13:50:04 GMT)
Full text and
rfc822 format available.
Message #190 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
> Date: Mon, 21 Oct 2024 09:52:02 +0800
>
> I think the next thing to do is try the rebase from the command line.
> So, please use the debugging version of my patch to create the special
> "amend!" commit again, or use the one from yesterday if you still have
> it, or you could manually create it by passing --allow-empty to 'git
> commit'.
>
> Then, at the command line, use
>
> git rebase --autosquash -i REV~1
>
> where REV is the sha1 of the commit whose message you intend to edit.
>
> This should open an editor with a rebase plan in which you see
>
> - the commit whose message you wanted to change
> - followed by the special amend! commit
> - followed by any other commits following the one whose message you wanted to change
>
> i.e. the amend! commit has been pulled down in history to just after the
> commit whose message you want to cahnge.
>
> Just save and exit without making any changes to the plan.
>
> After that, the whole process should be complete: you should have the
> same commits as previously except the message should have been updated,
> and sha1 hashes will have changed.
>
> Let me know if this all works.
I'm not sure I understood you correctly, but if I perform all these
step, I'm back at the original tip, with the original log message, not
the edited one.
But let me describe what I did and saw, to make sure I did it
correctly.
> git rebase --autosquash -i REV~1
>
> where REV is the sha1 of the commit whose message you intend to edit.
At this point "git log" shows a revision with my amended log message
(let's call this AMENDED-REV), followed by the revision with the
original log message (let's call it ORIG-REV). So my Git command
looks like this:
git rebase --autosquash -i ORIG-REV~1
After this step, I see this in the editor (where I indented the text
by 2 columns):
pick bf73d7e Foobar with some Unicode אבגד ą ě č
# pick bc3c567b2b136d040fd13373b6594c1ec026fec6 Foobar with some Unicode אבגד ą ě č
# Rebase 6c6ea73..bc3c567 onto 6c6ea73 (1 command)
#
# Commands:
# p, pick = use commit
# r, reword = use commit, but edit the commit message
# e, edit = use commit, but stop for amending
# s, squash = use commit, but meld into previous commit
# f, fixup = like "squash", but discard this commit's log message
# x, exec = run command (the rest of the line) using shell
# d, drop = remove commit
#
# These lines can be re-ordered; they are executed from top to bottom.
#
# If you remove a line here THAT COMMIT WILL BE LOST.
#
# However, if you remove everything, the rebase will be aborted.
#
# Note that empty commits are commented out
Here bf73d7e is ORIG-REV and bc3c567b is AMENDED-REV. At this point,
I just type "C-x #", since you said "just save and exit" (of course,
saving is not needed, since I didn't edit the buffer).
After that, I see in the shell window:
$ git rebase --autosquash -i bf73d7e036c5d80cdd112f34255a0ab1ea697c07~1
Waiting for Emacs...
Successfully rebased and updated refs/heads/master.
But "git log" shows the original log message for ORIG-REV.
Does this mean it didn't work? or did I misinterpret the steps?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 19:58:01 GMT)
Full text and
rfc822 format available.
Message #193 received at 64055 <at> debbugs.gnu.org (full text, mbox):
On 21/10/2024 08:46, Sean Whitton wrote:
> On Mon 21 Oct 2024 at 08:39am +03, Eli Zaretskii wrote:
>
>> Which aspects of "rewriting history" you consider Git-specific? I
>> think any dVCS has the same issues with that, and supports similar
>> workflows, so the concept is relevant to all of them.
> Mainly the terminology.
>
> If others think it is fine to go ahead and call the defcustom
> vc-allow-rewriting-history, I am happy to do so.
Speaking of Git terminology, "rewriting history" can refer to both
rewriting published commits (often frowned upon) and rewriting local
history (can be fine and is often encouraged). The proposed name seems
to conflate the two (although the docstring does clarify that it's
referring to the former).
In practice, this also often depends on the upstream branch - e.g. a
branch "feature/xyz" worked on a single developer might be fine with
force-pushes.
I guess my point was we could do with only a prompt warning the user
that the published history will be rewritten (proceeding on if they
agree). Showing an error is a safer choice, but I suppose then the error
message could reference the option to customize.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Mon, 21 Oct 2024 20:10:01 GMT)
Full text and
rfc822 format available.
Message #196 received at 64055 <at> debbugs.gnu.org (full text, mbox):
On 20/10/2024 08:19, Sean Whitton wrote:
> + (nothing-staged
> + (zerop
> + (vc-git-command nil t nil "diff" "--cached" "--quiet"))))
Testing it a little more, this seems insufficient because rebase aborts
on non-staged changes too. This seems to cover both cases:
(no-changes
(zerop
(vc-git-command nil t nil "diff" "HEAD" "--quiet"))))
Regarding the command's latency it seems to be distributed like this in
my current example:
allowed rewrite?
Elapsed time: 0.028043s
stash push maybe
Elapsed time: 0.072001s
rebase autosquash
Elapsed time: 0.047211s
stash pop
Elapsed time: 0.032561s
So all steps are somewhat expensive, but stashing is #1 in that. Overall
it seems fine, not a problem for the interactive command.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 22 Oct 2024 08:27:02 GMT)
Full text and
rfc822 format available.
Message #199 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
Thanks for trying the manual rebase.
On Mon 21 Oct 2024 at 04:48pm +03, Eli Zaretskii wrote:
> But let me describe what I did and saw, to make sure I did it
> correctly.
>
>> git rebase --autosquash -i REV~1
>>
>> where REV is the sha1 of the commit whose message you intend to edit.
>
> At this point "git log" shows a revision with my amended log message
> (let's call this AMENDED-REV), followed by the revision with the
> original log message (let's call it ORIG-REV). So my Git command
> looks like this:
>
> git rebase --autosquash -i ORIG-REV~1
>
> After this step, I see this in the editor (where I indented the text
> by 2 columns):
>
> pick bf73d7e Foobar with some Unicode אבגד ą ě č
> # pick bc3c567b2b136d040fd13373b6594c1ec026fec6 Foobar with some Unicode אבגד ą ě č
There are two things about this output that are unexpected to me:
- The first line of the AMENDED-REV message does not appear to start
with "amend!". I thought that you confirmed in a previous message
that the commit messsage of AMENDED-REV did have this structure:
amend! <ORIG-REV sha1>
<AMENDED-REV amended message>
- The line for the AMENDED-REV message is commented out. That means
that git-rebase proposes to drop the commit. I would guess that git
wants to drop it because it introduces no content changes.
> Does this mean it didn't work? or did I misinterpret the steps?
I think that there are two possibilities. If you lost the magic
"amend!" commit then please try again with an AMENDED-REV whose commit
message has the special "amend!" structure.
You can create one manually by passing --allow-empty to git-commit, or
you can use the previous version of my patch with everything except the
call to git-commit commented out -- here's the diff doing the
commenting-out again, if you need it.
On the other hand, if your AMENDED-REV does have the special "amend!"
structure, then I think we have to conclude that the older Git you have
does not support these special "amend!" commits at all, and so we can't
support it for log-view-modify-change-comment.
--8<---------------cut here---------------start------------->8---
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 59d5386bc72..d2ada63f71e 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -2042,8 +2042,8 @@ vc-git-modify-change-comment
;; % git commit --allow-empty -m...
;; % git rebase --autosquash -i REV~1
;; % git stash pop
- (unless nothing-staged
- (vc-git-command nil 0 nil "stash" "push"))
+ ;; (unless nothing-staged
+ ;; (vc-git-command nil 0 nil "stash" "push"))
(unwind-protect
(progn
(when (cl-intersection '("--author" "--date") args
@@ -2064,12 +2064,15 @@ vc-git-modify-change-comment
(list "-m" message))
args)))
(when (and msg-file (file-exists-p msg-file))
- (delete-file msg-file)))
- (with-environment-variables (("GIT_SEQUENCE_EDITOR" "true"))
- (vc-git-command nil 0 nil "rebase" "--autosquash" "-i"
- (format "%s~1" rev)))
- (unless nothing-staged
- (vc-git-command nil 0 nil "stash" "pop" "--index"))))
+ ;; (delete-file msg-file)
+ ))
+ ;; (with-environment-variables (("GIT_SEQUENCE_EDITOR" "true"))
+ ;; (vc-git-command nil 0 nil "rebase" "--autosquash" "-i"
+ ;; (format "%s~1" rev)))
+ ;; (unless nothing-staged
+ ;; (vc-git-command nil 0 nil "stash" "pop" "--index"))
+ (message "temporary file is: %s" msg-file)
+ ))
(defvar vc-git-extra-menu-map
(let ((map (make-sparse-keymap)))
--8<---------------cut here---------------end--------------->8---
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 22 Oct 2024 08:31:01 GMT)
Full text and
rfc822 format available.
Message #202 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Mon 21 Oct 2024 at 08:56pm +01, Dmitry Gutov wrote:
> On 21/10/2024 08:46, Sean Whitton wrote:
>> On Mon 21 Oct 2024 at 08:39am +03, Eli Zaretskii wrote:
>>
>>> Which aspects of "rewriting history" you consider Git-specific? I
>>> think any dVCS has the same issues with that, and supports similar
>>> workflows, so the concept is relevant to all of them.
>> Mainly the terminology.
>> If others think it is fine to go ahead and call the defcustom
>> vc-allow-rewriting-history, I am happy to do so.
>
> Speaking of Git terminology, "rewriting history" can refer to both rewriting
> published commits (often frowned upon) and rewriting local history (can be
> fine and is often encouraged). The proposed name seems to conflate the two
> (although the docstring does clarify that it's referring to the former).
>
> In practice, this also often depends on the upstream branch - e.g. a branch
> "feature/xyz" worked on a single developer might be fine with force-pushes.
Yeah. I'll use vc-allow-rewriting-published-history. It's not perfect
but it does a better job covering these various issues.
> I guess my point was we could do with only a prompt warning the user that the
> published history will be rewritten (proceeding on if they agree). Showing an
> error is a safer choice, but I suppose then the error message could reference
> the option to customize.
I think that's what's implemented, except for referencing the option
name, which I'll add, thanks.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 22 Oct 2024 08:32:02 GMT)
Full text and
rfc822 format available.
Message #205 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Mon 21 Oct 2024 at 09:08pm +01, Dmitry Gutov wrote:
> On 20/10/2024 08:19, Sean Whitton wrote:
>> + (nothing-staged
>> + (zerop
>> + (vc-git-command nil t nil "diff" "--cached" "--quiet"))))
>
> Testing it a little more, this seems insufficient because rebase aborts on
> non-staged changes too. This seems to cover both cases:
>
> (no-changes
> (zerop
> (vc-git-command nil t nil "diff" "HEAD" "--quiet"))))
Thanks. I forgot that --autostash covers that case already.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 22 Oct 2024 13:29:01 GMT)
Full text and
rfc822 format available.
Message #208 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
> Date: Tue, 22 Oct 2024 16:25:52 +0800
>
> > At this point "git log" shows a revision with my amended log message
> > (let's call this AMENDED-REV), followed by the revision with the
> > original log message (let's call it ORIG-REV). So my Git command
> > looks like this:
> >
> > git rebase --autosquash -i ORIG-REV~1
> >
> > After this step, I see this in the editor (where I indented the text
> > by 2 columns):
> >
> > pick bf73d7e Foobar with some Unicode אבגד ą ě č
> > # pick bc3c567b2b136d040fd13373b6594c1ec026fec6 Foobar with some Unicode אבגד ą ě č
>
> There are two things about this output that are unexpected to me:
>
> - The first line of the AMENDED-REV message does not appear to start
> with "amend!". I thought that you confirmed in a previous message
> that the commit messsage of AMENDED-REV did have this structure:
>
> amend! <ORIG-REV sha1>
>
> <AMENDED-REV amended message>
It isn't there because you didn't tell me to include it. The original
commit was lost when I tried to perform the steps the previous time,
so I had to reproduce it, and I used the last part of this step to
produce it:
> So, please use the debugging version of my patch to create the special
> "amend!" commit again, or use the one from yesterday if you still have
> it, or you could manually create it by passing --allow-empty to 'git
> commit'.
IOW, I "manually created it by passing --allow-empty to 'git commit'",
but since you didn't say what log message to give, I used just some
random text, and it didn't include "amend!". If the log message I use
in this step must have some specific structure or content, please give
more detailed instructions.
> - The line for the AMENDED-REV message is commented out. That means
> that git-rebase proposes to drop the commit. I would guess that git
> wants to drop it because it introduces no content changes.
>
> > Does this mean it didn't work? or did I misinterpret the steps?
>
> I think that there are two possibilities. If you lost the magic
> "amend!" commit then please try again with an AMENDED-REV whose commit
> message has the special "amend!" structure.
I will, once you give the detailed instructions for how to do it from
the shell prompt.
> You can create one manually by passing --allow-empty to git-commit, or
> you can use the previous version of my patch with everything except the
> call to git-commit commented out -- here's the diff doing the
> commenting-out again, if you need it.
The diff no longer applies to the current master. And I prefer doing
that manually anyway. So let's have those instructions in full
detail.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 22 Oct 2024 13:44:02 GMT)
Full text and
rfc822 format available.
Message #211 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Tue 22 Oct 2024 at 04:27pm +03, Eli Zaretskii wrote:
> It isn't there because you didn't tell me to include it. The original
> commit was lost when I tried to perform the steps the previous time,
> so I had to reproduce it, and I used the last part of this step to
> produce it:
>
>> So, please use the debugging version of my patch to create the special
>> "amend!" commit again, or use the one from yesterday if you still have
>> it, or you could manually create it by passing --allow-empty to 'git
>> commit'.
>
> IOW, I "manually created it by passing --allow-empty to 'git commit'",
> but since you didn't say what log message to give, I used just some
> random text, and it didn't include "amend!". If the log message I use
> in this step must have some specific structure or content, please give
> more detailed instructions.
> [...]
> The diff no longer applies to the current master. And I prefer doing
> that manually anyway. So let's have those instructions in full
> detail.
Okay, this should do it:
- Manually create a temporary file somewhere outside the repository;
I'll refer to its absolute file name as TEMP.
- Put the commit message for ORIG-REV into the temporary file.
- Modify that commit message to include the Unicode, or whatever.
- Prepend "amend! <sha1 of ORIG-REV>\n\n". So for example:
--8<---------------cut here---------------start------------->8---
amend! 212cf3125611b123707feac6f7ffd55a230bc568
Make all the entries in 'eshell-parse-argument-hook' named functions
* lisp/eshell/esh-arg.el (eshell-parse-number, eshell-parse-non-special)
(eshell-parse-whitespace, eshell-parse-comment): New functions...
(eshell-parse-argument-hook): ... use them. אבגד ą ě č
--8<---------------cut here---------------end--------------->8---
- Save the temporary file.
- Ensure there are no staged changes.
('git reset' with no arguments should do it)
- git commit --allow-empty -F TEMP
- git rebase --autosquash -i ORIG-REV~1
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 22 Oct 2024 14:22:02 GMT)
Full text and
rfc822 format available.
Message #214 received at 64055 <at> debbugs.gnu.org (full text, mbox):
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: 64055 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Morgan.J.Smith <at> outlook.com
> Date: Tue, 22 Oct 2024 21:42:33 +0800
>
> - Manually create a temporary file somewhere outside the repository;
> I'll refer to its absolute file name as TEMP.
>
> - Put the commit message for ORIG-REV into the temporary file.
>
> - Modify that commit message to include the Unicode, or whatever.
>
> - Prepend "amend! <sha1 of ORIG-REV>\n\n". So for example:
>
> --8<---------------cut here---------------start------------->8---
> amend! 212cf3125611b123707feac6f7ffd55a230bc568
>
> Make all the entries in 'eshell-parse-argument-hook' named functions
>
> * lisp/eshell/esh-arg.el (eshell-parse-number, eshell-parse-non-special)
> (eshell-parse-whitespace, eshell-parse-comment): New functions...
> (eshell-parse-argument-hook): ... use them. אבגד ą ě č
> --8<---------------cut here---------------end--------------->8---
>
> - Save the temporary file.
>
> - Ensure there are no staged changes.
> ('git reset' with no arguments should do it)
>
> - git commit --allow-empty -F TEMP
>
> - git rebase --autosquash -i ORIG-REV~1
This last step shows the following (again, indented 2 columns by me):
pick bf73d7e Foobar with some Unicode אבגד ą ě č
# pick d796890ee8cc77ac899954c3e94e5257c5f72615 amend! bf73d7e036c5d80cdd112f34255a0ab1ea697c07
# Rebase 6c6ea73..d796890 onto 6c6ea73 (1 command)
#
# Commands:
# p, pick = use commit
# r, reword = use commit, but edit the commit message
# e, edit = use commit, but stop for amending
# s, squash = use commit, but meld into previous commit
# f, fixup = like "squash", but discard this commit's log message
# x, exec = run command (the rest of the line) using shell
# d, drop = remove commit
#
# These lines can be re-ordered; they are executed from top to bottom.
#
# If you remove a line here THAT COMMIT WILL BE LOST.
#
# However, if you remove everything, the rebase will be aborted.
#
# Note that empty commits are commented out
And if I type "C-x #" to exit, without changing the above in any way
(since you didn't tell me to change anything), I'm back at ORIG-REV
with its original log message. IOW, the editing of the commit log
message failed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Tue, 22 Oct 2024 14:54:02 GMT)
Full text and
rfc822 format available.
Message #217 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Tue 22 Oct 2024 at 05:20pm +03, Eli Zaretskii wrote:
> This last step shows the following (again, indented 2 columns by me):
>
> pick bf73d7e Foobar with some Unicode אבגד ą ě č
> # pick d796890ee8cc77ac899954c3e94e5257c5f72615 amend!
> bf73d7e036c5d80cdd112f34255a0ab1ea697c07
>
> # Rebase 6c6ea73..d796890 onto 6c6ea73 (1 command)
> #
> # Commands:
> # p, pick = use commit
> # r, reword = use commit, but edit the commit message
> # e, edit = use commit, but stop for amending
> # s, squash = use commit, but meld into previous commit
> # f, fixup = like "squash", but discard this commit's log message
> # x, exec = run command (the rest of the line) using shell
> # d, drop = remove commit
> #
> # These lines can be re-ordered; they are executed from top to bottom.
> #
> # If you remove a line here THAT COMMIT WILL BE LOST.
> #
> # However, if you remove everything, the rebase will be aborted.
> #
> # Note that empty commits are commented out
>
> And if I type "C-x #" to exit, without changing the above in any way
> (since you didn't tell me to change anything), I'm back at ORIG-REV
> with its original log message. IOW, the editing of the commit log
> message failed.
Thanks. I think this concludes the testing on old Git on Windows.
There are two conclusions:
- the special handling for MS-Windows character encoding issues works
- the special handling for old Git cannot work, because the older Git
doesn't recognise the "amend!" commits as special.
So, I'll restore the old sequence of git commands, move the defcustom
into vc.git and call it vc-allow-rewriting-published-history, and
install the change. (It's late here, so, in the morning.)
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sat, 26 Oct 2024 02:00:02 GMT)
Full text and
rfc822 format available.
Message #220 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hi Sean,
On 20/10/2024 08:19, Sean Whitton wrote:
> +Many Git commands can change your copy of published change history
> +without warning. If VC commands detect that this could happen, they
> +will stop. You can customize this variable to permit rewriting history
> +even though Emacs thinks it is dangerous.
> +
> +So far, this applies only to the 'log-view-modify-change-comment'
> +command.
Coming back to this text - do you think the first and the last sentences
might mismatch? First we talk about "many Git commands", next sentence
"VC commands", plural. And in the end we say this only applies to one.
Maybe we can rephrase it like this:
Some VCS commands can change your copy of published change history
without warning. In VC we try to detect before that happens, and stop.
You can customize this variable to permit rewriting history
even though Emacs thinks it is dangerous.
So far, this applies only to using 'e' from Log View mode for Git.
BTW, there is another existing command which can end up changing
published history: 'vc-git-log-edit-toggle-amend'. I wonder what will be
our plan for it. Maybe we just add the same check there.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sat, 26 Oct 2024 03:13:01 GMT)
Full text and
rfc822 format available.
Message #223 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sat 26 Oct 2024 at 04:58am +03, Dmitry Gutov wrote:
> Hi Sean,
>
> On 20/10/2024 08:19, Sean Whitton wrote:
>> +Many Git commands can change your copy of published change history
>> +without warning. If VC commands detect that this could happen, they
>> +will stop. You can customize this variable to permit rewriting history
>> +even though Emacs thinks it is dangerous.
>> +
>> +So far, this applies only to the 'log-view-modify-change-comment'
>> +command.
>
> Coming back to this text - do you think the first and the last sentences might
> mismatch? First we talk about "many Git commands", next sentence "VC
> commands", plural. And in the end we say this only applies to one.
>
> Maybe we can rephrase it like this:
>
> Some VCS commands can change your copy of published change history
> without warning. In VC we try to detect before that happens, and stop.
> You can customize this variable to permit rewriting history
> even though Emacs thinks it is dangerous.
>
> So far, this applies only to using 'e' from Log View mode for Git.
Whether or not there is a strict mismatch, I like your new text more.
Please install it.
> BTW, there is another existing command which can end up changing
> published history: 'vc-git-log-edit-toggle-amend'. I wonder what will
> be our plan for it. Maybe we just add the same check there.
Thanks for reminding me about this. Yes, I think it should be fine to
just add (vc-git--assert-allowed-rewrite "HEAD") or similar. Would you
like to do it along with your NEWS change? I guess, then, dropping the
"So far ..." sentence.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 27 Oct 2024 01:06:02 GMT)
Full text and
rfc822 format available.
Message #226 received at 64055 <at> debbugs.gnu.org (full text, mbox):
On 26/10/2024 06:12, Sean Whitton wrote:
>> Maybe we can rephrase it like this:
>>
>> Some VCS commands can change your copy of published change history
>> without warning. In VC we try to detect before that happens, and stop.
>> You can customize this variable to permit rewriting history
>> even though Emacs thinks it is dangerous.
>>
>> So far, this applies only to using 'e' from Log View mode for Git.
> Whether or not there is a strict mismatch, I like your new text more.
> Please install it.
>
>> BTW, there is another existing command which can end up changing
>> published history: 'vc-git-log-edit-toggle-amend'. I wonder what will
>> be our plan for it. Maybe we just add the same check there.
> Thanks for reminding me about this. Yes, I think it should be fine to
> just add (vc-git--assert-allowed-rewrite "HEAD") or similar. Would you
> like to do it along with your NEWS change? I guess, then, dropping the
> "So far ..." sentence.
Great! Pushed in 2030b8c7f24.
Speaking of the description in NEWS, completion just reminded me that we
also have vc-hg-log-edit-toggle-amend, maybe we could also do something
about it.
I also got to thinking about the older VCSes supporting
'modify-change-comment'. OT1H it also modifies "published history" in
those instances. OT2H it does that in a way that won't lead to later
conflict, because none of those systems are distributed. So I suppose we
don't need to worry about them.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#64055; Package
emacs.
(Sun, 27 Oct 2024 02:27:02 GMT)
Full text and
rfc822 format available.
Message #229 received at 64055 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sun 27 Oct 2024 at 03:05am +02, Dmitry Gutov wrote:
> On 26/10/2024 06:12, Sean Whitton wrote:
>>> Maybe we can rephrase it like this:
>>>
>>> Some VCS commands can change your copy of published change history
>>> without warning. In VC we try to detect before that happens, and stop.
>>> You can customize this variable to permit rewriting history
>>> even though Emacs thinks it is dangerous.
>>>
>>> So far, this applies only to using 'e' from Log View mode for Git.
>> Whether or not there is a strict mismatch, I like your new text more.
>> Please install it.
>>
>>> BTW, there is another existing command which can end up changing
>>> published history: 'vc-git-log-edit-toggle-amend'. I wonder what will
>>> be our plan for it. Maybe we just add the same check there.
>> Thanks for reminding me about this. Yes, I think it should be fine to
>> just add (vc-git--assert-allowed-rewrite "HEAD") or similar. Would you
>> like to do it along with your NEWS change? I guess, then, dropping the
>> "So far ..." sentence.
>
> Great! Pushed in 2030b8c7f24.
Thanks.
> Speaking of the description in NEWS, completion just reminded me that
> we also have vc-hg-log-edit-toggle-amend, maybe we could also do
> something about it.
Yes, would be good to.
> I also got to thinking about the older VCSes supporting
> 'modify-change-comment'. OT1H it also modifies "published history" in those
> instances. OT2H it does that in a way that won't lead to later conflict,
> because none of those systems are distributed. So I suppose we don't need to
> worry about them.
Yes, I think that's right.
--
Sean Whitton
Forcibly Merged 38343 64055.
Request was from
Sean Whitton <spwhitton <at> spwhitton.name>
to
control <at> debbugs.gnu.org.
(Tue, 04 Mar 2025 04:18:02 GMT)
Full text and
rfc822 format available.
Disconnected #38343 from all other report(s).
Request was from
Sean Whitton <spwhitton <at> spwhitton.name>
to
control <at> debbugs.gnu.org.
(Tue, 04 Mar 2025 04:27:02 GMT)
Full text and
rfc822 format available.
This bug report was last modified 103 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.