- From:
- "Lorenz (xha)" <me@xha.li>
- Subject:
- Re: [PATCH] got histedit: make mesg behave like pick
- To:
- gameoftrees@openbsd.org
- Date:
- Tue, 24 Oct 2023 15:47:56 +0200
Download raw body.
Thread
[PATCH] got histedit: make mesg behave like pick
Download raw body.
[PATCH] got histedit: make mesg behave like pick
i realize now that this has a bug:
```
mesg <commit>
edit <commit>
```
it will open the editor at least twice to edit the same commit message
(for the `mesg` command). this is because we now parse the histedit
script two times:
- when intially closing the editor that had the histedit script open
- when continuing the edit using `histedit -c`
there is no way to save the log message without completly changing how
histedit works afaik...
On Tue, Oct 24, 2023 at 12:43:13PM +0200, Lorenz (xha) wrote:
> hi,
>
> as discussed on irc, i propose to change the histedit command `mesg`
> to behave like the `pick` command but to edit the commit message in
> $EDITOR. this is how that would look like:
>
> ```
> drop <commit>
> fold <commit>
> # same as `pick` but edit log message in $EDITOR
> mesg <commit>
> ```
>
> these are the reasons for this change:
> - having fold operate on the next commit and mesg on the previous one is
> confusing. there is no good place to put mesg
> - probably none is using the scripting functionality of mesg outside of
> the regression tests. you can still use a editor.sh script
> - it is more intuitive to use `mesg` like `pick`
>
> the regress tests are passing for me. as expected, i had to add some
> editor.sh "hacks".
>
> diff /home/lorenz/src/got
> commit - 41208069f572694ef8847dbe9d9a47337f7083bd
> path + /home/lorenz/src/got
> blob - 52eeeda8df9fb52a8581faa28c2d29c19ca5a2be
> file + got/got.1
> --- got/got.1
> +++ got/got.1
> @@ -3016,18 +3016,7 @@ Regular editing of history must eventually be resumed
> .It Cm fold Ar commit Ta Combine the specified commit with the next commit
> listed further below that will be used.
> .It Cm drop Ar commit Ta Remove this commit from the edited history.
> -.It Cm mesg Oo Ar log-message Oc Ta Create a new log message for the commit of
> -a preceding
> -.Cm pick
> -or
> -.Cm edit
> -command on the previous line of the histedit script.
> -The optional
> -.Ar log-message
> -argument provides a new single-line log message to use.
> -If the
> -.Ar log-message
> -argument is omitted, open an editor where a new log message can be written.
> +.It Cm mesg Ar commit Ta Use the specified commit but edit the log message.
> .El
> .Pp
> Every commit in the history being edited must be mentioned in the script.
> blob - 73812ddbb0f2e3a0fee927fc0ae5b873c04e385a
> file + got/got.c
> --- got/got.c
> +++ got/got.c
> @@ -11696,8 +11696,7 @@ static const struct got_histedit_cmd {
> { GOT_HISTEDIT_FOLD, "fold", "combine with next commit that will "
> "be used" },
> { GOT_HISTEDIT_DROP, "drop", "remove commit from history" },
> - { GOT_HISTEDIT_MESG, "mesg",
> - "single-line log message for commit above (open editor if empty)" },
> + { GOT_HISTEDIT_MESG, "mesg", "use commit but edit the log message" },
> };
>
> struct got_histedit_list_entry {
> @@ -11760,16 +11759,11 @@ histedit_write_commit_list(struct got_object_id_queue
> histedit_cmd = "edit";
> else if (fold_only && STAILQ_NEXT(qid, entry) != NULL)
> histedit_cmd = "fold";
> + else if (edit_logmsg_only)
> + histedit_cmd = "mesg";
> err = histedit_write_commit(&qid->id, histedit_cmd, f, repo);
> if (err)
> break;
> - if (edit_logmsg_only) {
> - int n = fprintf(f, "%c\n", GOT_HISTEDIT_MESG);
> - if (n < 0) {
> - err = got_ferror(f, GOT_ERR_IO);
> - break;
> - }
> - }
> }
>
> return err;
> @@ -11986,7 +11980,7 @@ histedit_parse_list(struct got_histedit_list *histedit
> char *line = NULL, *p, *end;
> size_t i, linesize = 0;
> ssize_t linelen;
> - int lineno = 0, lastcmd = -1;
> + int lineno = 0;
> const struct got_histedit_cmd *cmd;
> struct got_object_id *commit_id = NULL;
> struct got_histedit_list_entry *hle = NULL;
> @@ -12026,37 +12020,15 @@ histedit_parse_list(struct got_histedit_list *histedit
> }
> while (isspace((unsigned char)p[0]))
> p++;
> - if (cmd->code == GOT_HISTEDIT_MESG) {
> - if (lastcmd != GOT_HISTEDIT_PICK &&
> - lastcmd != GOT_HISTEDIT_EDIT) {
> - err = got_error(GOT_ERR_HISTEDIT_CMD);
> - break;
> - }
> - if (p[0] == '\0') {
> - err = histedit_edit_logmsg(hle, repo);
> - if (err)
> - break;
> - } else {
> - hle->logmsg = strdup(p);
> - if (hle->logmsg == NULL) {
> - err = got_error_from_errno("strdup");
> - break;
> - }
> - }
> - lastcmd = cmd->code;
> - continue;
> - } else {
> - end = p;
> - while (end[0] && !isspace((unsigned char)end[0]))
> - end++;
> - *end = '\0';
> -
> - err = got_object_resolve_id_str(&commit_id, repo, p);
> - if (err) {
> - /* override error code */
> - err = histedit_syntax_error(lineno);
> - break;
> - }
> + end = p;
> + while (end[0] && !isspace((unsigned char)end[0]))
> + end++;
> + *end = '\0';
> + err = got_object_resolve_id_str(&commit_id, repo, p);
> + if (err) {
> + /* override error code */
> + err = histedit_syntax_error(lineno);
> + break;
> }
> hle = malloc(sizeof(*hle));
> if (hle == NULL) {
> @@ -12068,7 +12040,12 @@ histedit_parse_list(struct got_histedit_list *histedit
> hle->logmsg = NULL;
> commit_id = NULL;
> TAILQ_INSERT_TAIL(histedit_cmds, hle, entry);
> - lastcmd = cmd->code;
> +
> + if (cmd->code == GOT_HISTEDIT_MESG) {
> + err = histedit_edit_logmsg(hle, repo);
> + if (err)
> + break;
> + }
> }
>
> free(line);
> @@ -12270,15 +12247,6 @@ histedit_save_list(struct got_histedit_list *histedit_
> repo);
> if (err)
> break;
> -
> - if (hle->logmsg) {
> - int n = fprintf(f, "%c %s\n",
> - GOT_HISTEDIT_MESG, hle->logmsg);
> - if (n < 0) {
> - err = got_ferror(f, GOT_ERR_IO);
> - break;
> - }
> - }
> }
> done:
> if (f && fclose(f) == EOF && err == NULL)
> @@ -12416,6 +12384,7 @@ show_histedit_progress(struct got_commit_object *commi
> switch (hle->cmd->code) {
> case GOT_HISTEDIT_PICK:
> case GOT_HISTEDIT_EDIT:
> + case GOT_HISTEDIT_MESG:
> printf("%s -> %s: %s\n", old_id_str,
> new_id_str ? new_id_str : "no-op change", logmsg);
> break;
> blob - a8c20c791a546fc257c69e471cb4ff9e59cd4d66
> file + regress/cmdline/histedit.sh
> --- regress/cmdline/histedit.sh
> +++ regress/cmdline/histedit.sh
> @@ -491,13 +491,22 @@ test_histedit_fold() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/committing folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> echo "fold $old_commit1" > $testroot/histedit-script
> echo "drop $old_commit2" >> $testroot/histedit-script
> echo "pick $old_commit3" >> $testroot/histedit-script
> - echo "mesg committing folded changes" >> $testroot/histedit-script
>
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout)
>
> local new_commit1=`git_show_parent_commit $testroot/repo`
> local new_commit2=`git_show_head $testroot/repo`
> @@ -603,7 +612,6 @@ test_histedit_edit() {
> fi
>
> echo "edit $old_commit1" > $testroot/histedit-script
> - echo "mesg committing changes" >> $testroot/histedit-script
> echo "pick $old_commit2" >> $testroot/histedit-script
>
> (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> @@ -650,8 +658,20 @@ test_histedit_edit() {
> fi
>
> (cd $testroot/wt && got unstage alpha > /dev/null)
> - (cd $testroot/wt && got histedit -c > $testroot/stdout)
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/committing changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -c > $testroot/stdout)
> +
> local new_commit1=`git_show_parent_commit $testroot/repo`
> local new_commit2=`git_show_head $testroot/repo`
>
> @@ -771,7 +791,7 @@ test_histedit_fold_last_commit() {
> test_done "$testroot" "$ret"
> }
>
> -test_histedit_missing_commit() {
> +test_histedit_missing_commit_pick() {
> local testroot=`test_init histedit_missing_commit`
>
> local orig_commit=`git_show_head $testroot/repo`
> @@ -795,7 +815,6 @@ test_histedit_missing_commit() {
> fi
>
> echo "pick $old_commit1" > $testroot/histedit-script
> - echo "mesg committing changes" >> $testroot/histedit-script
>
> (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> > $testroot/stdout 2> $testroot/stderr)
> @@ -818,6 +837,63 @@ test_histedit_missing_commit() {
> test_done "$testroot" "$ret"
> }
>
> +test_histedit_missing_commit_mesg() {
> + local testroot=`test_init histedit_missing_commit`
> +
> + local orig_commit=`git_show_head $testroot/repo`
> +
> + echo "modified alpha on master" > $testroot/repo/alpha
> + git -C $testroot/repo rm -q beta
> + echo "new file on master" > $testroot/repo/epsilon/new
> + git -C $testroot/repo add epsilon/new
> + git_commit $testroot/repo -m "committing changes"
> + local old_commit1=`git_show_head $testroot/repo`
> +
> + echo "modified zeta on master" > $testroot/repo/epsilon/zeta
> + git_commit $testroot/repo -m "committing to zeta on master"
> + local old_commit2=`git_show_head $testroot/repo`
> +
> + got checkout -c $orig_commit $testroot/repo $testroot/wt > /dev/null
> + ret=$?
> + if [ $ret -ne 0 ]; then
> + test_done "$testroot" "$ret"
> + return 1
> + fi
> +
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/committing folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> + echo "mesg $old_commit1" > $testroot/histedit-script
> +
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout \
> + 2>$testroot/stderr)
> +
> + ret=$?
> + if [ $ret -eq 0 ]; then
> + echo "histedit succeeded unexpectedly" >&2
> + test_done "$testroot" "1"
> + return 1
> + fi
> +
> + echo "got: commit $old_commit2 missing from histedit script" \
> + > $testroot/stderr.expected
> +
> + cmp -s $testroot/stderr.expected $testroot/stderr
> + ret=$?
> + if [ $ret -ne 0 ]; then
> + diff -u $testroot/stderr.expected $testroot/stderr
> + fi
> + test_done "$testroot" "$ret"
> +}
> +
> test_histedit_abort() {
> local testroot=`test_init histedit_abort`
>
> @@ -845,7 +921,6 @@ test_histedit_abort() {
> touch $testroot/wt/unversioned-file
>
> echo "edit $old_commit1" > $testroot/histedit-script
> - echo "mesg committing changes" >> $testroot/histedit-script
> echo "pick $old_commit2" >> $testroot/histedit-script
>
> (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> @@ -1051,7 +1126,6 @@ test_histedit_path_prefix_edit() {
> fi
>
> echo "edit $old_commit1" > $testroot/histedit-script
> - echo "mesg modified zeta" >> $testroot/histedit-script
>
> (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> > $testroot/stdout 2> $testroot/stderr)
> @@ -1120,8 +1194,19 @@ test_histedit_path_prefix_edit() {
> test_done "$testroot" "$ret"
> return 1
> fi
> +
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/modified zeta/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
>
> - (cd $testroot/wt && got histedit -c > $testroot/stdout)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -c > $testroot/stdout)
>
> local new_commit1=`git_show_head $testroot/repo`
> local short_new_commit1=`trim_obj_id 28 $new_commit1`
> @@ -1244,13 +1329,23 @@ test_histedit_fold_last_commit_swap() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/committing folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> # fold commit2 into commit1 (requires swapping commits)
> echo "fold $old_commit2" > $testroot/histedit-script
> - echo "pick $old_commit1" >> $testroot/histedit-script
> - echo "mesg committing folded changes" >> $testroot/histedit-script
> + echo "mesg $old_commit1" >> $testroot/histedit-script
>
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout 2> $testroot/stderr)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout \
> + 2> $testroot/stderr)
>
> ret=$?
> if [ $ret -ne 0 ]; then
> @@ -1470,13 +1565,22 @@ test_histedit_fold_add_delete() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> echo "fold $old_commit1" > $testroot/histedit-script
> echo "fold $old_commit2" >> $testroot/histedit-script
> echo "pick $old_commit3" >> $testroot/histedit-script
> - echo "mesg folded changes" >> $testroot/histedit-script
>
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout)
>
> local new_commit1=`git_show_head $testroot/repo`
>
> @@ -1565,12 +1669,21 @@ test_histedit_fold_edit_delete() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> echo "fold $old_commit1" > $testroot/histedit-script
> echo "pick $old_commit2" >> $testroot/histedit-script
> - echo "mesg folded changes" >> $testroot/histedit-script
>
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout)
>
> local new_commit1=`git_show_head $testroot/repo`
>
> @@ -1645,12 +1758,21 @@ test_histedit_fold_delete_add() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> echo "fold $old_commit1" > $testroot/histedit-script
> echo "pick $old_commit2" >> $testroot/histedit-script
> - echo "mesg folded changes" >> $testroot/histedit-script
>
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout)
>
> local new_commit1=`git_show_head $testroot/repo`
>
> @@ -2142,126 +2264,6 @@ EOF
> test_done "$testroot" $ret
> }
>
> -test_histedit_mesg_invalid() {
> - local testroot=`test_init mesg_invalid`
> -
> - local orig_commit=`git_show_head $testroot/repo`
> -
> - echo "modified alpha on master" > $testroot/repo/alpha
> - git -C $testroot/repo rm -q beta
> - echo "new file on master" > $testroot/repo/epsilon/new
> - git -C $testroot/repo add epsilon/new
> - git_commit $testroot/repo -m 'committing changes'
> - local old_commit1=`git_show_head $testroot/repo`
> -
> - echo "modified zeta on master" > $testroot/repo/epsilon/zeta
> - git_commit $testroot/repo -m 'committing to zeto on master'
> - local old_commit2=`git_show_head $testroot/repo`
> -
> - got checkout -c $orig_commit $testroot/repo $testroot/wt > /dev/null
> - ret=$?
> - if [ $ret -ne 0 ]; then
> - test_done "$testroot" $ret
> - return 1
> - fi
> -
> - # try with a leading mesg
> -
> - echo "mesg something something" > $testroot/histedit-script
> - echo "pick $old_commit1" >> $testroot/histedit-script
> - echo "pick $old_commit2" >> $testroot/histedit-script
> -
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout 2> $testroot/stderr)
> - ret=$?
> - if [ $ret -eq 0 ]; then
> - echo "histedit succeeded unexpectedly" >&2
> - test_done "$testroot" 1
> - return 1
> - fi
> -
> - echo "got: bad histedit command" > $testroot/stderr.expected
> - cmp -s $testroot/stderr.expected $testroot/stderr
> - ret=$?
> - if [ $ret -ne 0 ]; then
> - diff -u $testroot/stderr.expected $testroot/stderr
> - test_done "$testroot" $ret
> - return 1
> - fi
> -
> - # try again with mesg -> mesg
> -
> - echo "pick $old_commit1" > $testroot/histedit-script
> - echo "mesg something something" >> $testroot/histedit-script
> - echo "mesg something something else" >> $testroot/histedit-script
> - echo "pick $old_commit2" >> $testroot/histedit-script
> -
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout 2> $testroot/stderr)
> - ret=$?
> - if [ $ret -eq 0 ]; then
> - echo "histedit succeeded unexpectedly" >&2
> - test_done "$testroot" 1
> - return 1
> - fi
> -
> - echo "got: bad histedit command" > $testroot/stderr.expected
> - cmp -s $testroot/stderr.expected $testroot/stderr
> - ret=$?
> - if [ $ret -ne 0 ]; then
> - diff -u $testroot/stderr.expected $testroot/stderr
> - test_done "$testroot" $ret
> - return 1
> - fi
> -
> - # try again with drop -> mesg
> -
> - echo "drop $old_commit1" > $testroot/histedit-script
> - echo "mesg something something" >> $testroot/histedit-script
> - echo "pick $old_commit2" >> $testroot/histedit-script
> -
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout 2> $testroot/stderr)
> - ret=$?
> - if [ $ret -eq 0 ]; then
> - echo "histedit succeeded unexpectedly" >&2
> - test_done "$testroot" 1
> - return 1
> - fi
> -
> - echo "got: bad histedit command" > $testroot/stderr.expected
> - cmp -s $testroot/stderr.expected $testroot/stderr
> - ret=$?
> - if [ $ret -ne 0 ]; then
> - diff -u $testroot/stderr.expected $testroot/stderr
> - test_done "$testroot" $ret
> - return 1
> - fi
> -
> - # try again with fold -> mesg
> -
> - echo "fold $old_commit1" > $testroot/histedit-script
> - echo "mesg something something" >> $testroot/histedit-script
> - echo "pick $old_commit2" >> $testroot/histedit-script
> -
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout 2> $testroot/stderr)
> - ret=$?
> - if [ $ret -eq 0 ]; then
> - echo "histedit succeeded unexpectedly" >&2
> - test_done "$testroot" 1
> - return 1
> - fi
> -
> - echo "got: bad histedit command" > $testroot/stderr.expected
> - cmp -s $testroot/stderr.expected $testroot/stderr
> - ret=$?
> - if [ $ret -ne 0 ]; then
> - diff -u $testroot/stderr.expected $testroot/stderr
> - fi
> - test_done "$testroot" $ret
> -}
> -
> test_histedit_resets_committer() {
> local testroot=`test_init histedit_resets_committer`
> local orig_commit=`git_show_head $testroot/repo`
> @@ -2354,13 +2356,22 @@ test_histedit_umask() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/folding changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> echo fold $commit1 >$testroot/histedit-script
> echo fold $commit2 >>$testroot/histedit-script
> echo pick $commit3 >>$testroot/histedit-script
> - echo mesg folding changes >>$testroot/histedit-script
>
> # using a subshell to avoid clobbering global umask
> (umask 077 && cd "$testroot/wt" && \
> + env EDITOR="$testroot/editor.sh" VISUAL="$testroot/editor.sh" \
> got histedit -F "$testroot/histedit-script") >/dev/null
> ret=$?
>
> @@ -2604,7 +2615,8 @@ run_test test_histedit_drop
> run_test test_histedit_fold
> run_test test_histedit_edit
> run_test test_histedit_fold_last_commit
> -run_test test_histedit_missing_commit
> +run_test test_histedit_missing_commit_pick
> +run_test test_histedit_missing_commit_mesg
> run_test test_histedit_abort
> run_test test_histedit_path_prefix_drop
> run_test test_histedit_path_prefix_edit
> @@ -2619,7 +2631,6 @@ run_test test_histedit_fold_only
> run_test test_histedit_fold_only_empty_logmsg
> run_test test_histedit_edit_only
> run_test test_histedit_prepend_line
> -run_test test_histedit_mesg_invalid
> run_test test_histedit_resets_committer
> run_test test_histedit_umask
> run_test test_histedit_mesg_filemode_change
>
[PATCH] got histedit: make mesg behave like pick