threads / discuss / 48071

[GSoC] Some #leftoverbits for anyone looking for little projects

Subject: [GSoC] Some #leftoverbits for anyone looking for little projects

## tl;dr

13 messages between Mar 17, 2018 and Mar 23, 2022.

replies: 12people: 5as markdown or json

Ævar Arnfjörð Bjarmason· Mar 17, 2018, 21:20 UTC · lore

In lieu of sending a PR to https://git.github.io/SoC-2018-Microprojects/ I thought I'd list a few more suggestions, and hopefully others will chime in.

This is all TODO stuff I've been meaning to do myself, but wouldn't mind at all if someone else tackled.

I'm not interested in mentoring GSoC, but these are all small enough to need to special help from me (or anyone in particular), and if nobody picks them up I can refer back to this mail for my own use.

 * Having grep support the -o option like GNU grep et al.
   We have most of the code for this already in the form of our color
   hi-lighting, it would mostly just be a matter of "just print out the
   stuff you'd have colored", with the small exception that if you have
   more than one match on a line they should be printed out on their own
   lines.
 * Give "rebase -i" some option so when you "reword" the patch is
   included in the message.
   I keep going to the shell because I have no idea what change I'm
   describing.
 * Add more config IncludeIf conditions.
   Recently there was a mention on git-users to excend the includeIf
   statement to read config:
   https://groups.google.com/forum/?fromgroups#!searchin/git-users/includeif%7Csort:date/git-users/SHd506snwSk/UdVCsCILBwAJ
   Now that seems like a nasty circular dependency but there's other
   low-hanging fruit there, like make it match a given env name to a
   value (or glob?).
 * Add another set of GIT_{AUTHOR,COMMITTER}_{NAME,EMAIL} with lower
   priorities.
   There is a script at work which I have to manually blacklist which
   sets git author names & e-mails via LDAP for all logged in users via
   /etc/profile (and gets my name wrong)[1].
   It would be nice if git supported a way to do this that didn't either
   involve overriding everything (as the current env vars do) or munging
   the user's ~ config (ew!). I.e. the priority of these new env vars
   would come after reading from the config, not overriding the config
   as the current ones do. So it could be used to make a suggestion if
   no other value was found.
 * Write git-unpack-{refs,objects}
   I don't know if this is small enough (maybe the refs part?). This
   would give you purely loose objects & refs. This is a terrible idea
   for any "real" use, but very useful for testing.
   Now when I'm testing repack I need to keep an old copy of the repo
   around, because there's no easy way (that I know of) to pack things
   and then get back to loose object state. Ditto for packing refs.
 * I had a previous TODO list of "small" things at
   https://public-inbox.org/git/CACBZZX5wdnA-96e11edE7xRnAHo19RFTrZmqFQj-0ogLOJTncQ@mail.gmail.com/
1. At work like in so many companies LDAP is synced everywhere, but of
   course that means catering to the lowest common denominator. Last I
   heard attempts to give me a non-ASCII name (in the GEOS field) had
   failed because some phone or printer somewhere refused to accept it.
Matheus Tavares· May 20, 2019, 18:23 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [GSoC] Some #leftoverbits for anyone looking for little projects

Hi, Ævar
Show 5 quoted lines
> Give "rebase -i" some option so when you "reword" the patch is
> included in the message.
>
> I keep going to the shell because I have no idea what change I'm
> describing.

I have the same problem, so I wanted to try solving this. The patch bellow creates a "rebase.verboseCommit" configuration that includes a diff when rewording or squashing. I'd appreciate knowing your thoughts on it.

As Christian wisely pointed out to me, though, we can also achieve this behavior by setting "commit.verbose" to true. The only "downside" of it is that users cannot choose to see the diff only when rebasing. Despite of that, if we decide not to go with this patch, what do you think of adding a "commit.verbose" entry at git-rebase's man page? 

diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
index d98e32d812..ae50b3e05d 100644
--- a/Documentation/config/rebase.txt
+++ b/Documentation/config/rebase.txt
@@ -62,3 +62,8 @@ rebase.rescheduleFailedExec::
 	Automatically reschedule `exec` commands that failed. This only makes
 	sense in interactive mode (or when an `--exec` option was provided).
 	This is the same as specifying the `--reschedule-failed-exec` option.
+
+rebase.verboseCommit::
+	When rewording or squashing commits, during an interactive rebase, show
+	the commits' diff to help describe the modifications they bring. False
+	by default.
diff --git a/sequencer.c b/sequencer.c
index f88a97fb10..1596fc4cd0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -914,6 +914,7 @@ N_("you have staged changes in your working tree\n"
 #define CLEANUP_MSG (1<<3)
 #define VERIFY_MSG  (1<<4)
 #define CREATE_ROOT_COMMIT (1<<5)
+#define VERBOSE_COMMIT (1<<6)
 
 static int run_command_silent_on_success(struct child_process *cmd)
 {
@@ -1007,6 +1008,8 @@ static int run_git_commit(struct repository *r,
 		argv_array_push(&cmd.args, "-n");
 	if ((flags & AMEND_MSG))
 		argv_array_push(&cmd.args, "--amend");
+	if ((flags & VERBOSE_COMMIT))
+		argv_array_push(&cmd.args, "-v");
 	if (opts->gpg_sign)
 		argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
 	if (defmsg)
@@ -1782,7 +1785,7 @@ static int do_pick_commit(struct repository *r,
 	char *author = NULL;
 	struct commit_message msg = { NULL, NULL, NULL, NULL };
 	struct strbuf msgbuf = STRBUF_INIT;
-	int res, unborn = 0, allow;
+	int res, unborn = 0, allow, verbose_commit = 0;
 
 	if (opts->no_commit) {
 		/*
@@ -1843,6 +1846,9 @@ static int do_pick_commit(struct repository *r,
 		return error(_("cannot get commit message for %s"),
 			oid_to_hex(&commit->object.oid));
 
+	if (git_config_get_maybe_bool("rebase.verbosecommit", &verbose_commit) < 0)
+		warning("Invalid value for rebase.verboseCommit. Using 'false' instead.");
+
 	if (opts->allow_ff && !is_fixup(command) &&
 	    ((parent && oideq(&parent->object.oid, &head)) ||
 	     (!parent && unborn))) {
@@ -1853,6 +1859,8 @@ static int do_pick_commit(struct repository *r,
 		if (res || command != TODO_REWORD)
 			goto leave;
 		flags |= EDIT_MSG | AMEND_MSG | VERIFY_MSG;
+		if (verbose_commit)
+			flags |= VERBOSE_COMMIT;
 		msg_file = NULL;
 		goto fast_forward_edit;
 	}
@@ -1909,12 +1917,17 @@ static int do_pick_commit(struct repository *r,
 			author = get_author(msg.message);
 	}
 
-	if (command == TODO_REWORD)
+	if (command == TODO_REWORD) {
 		flags |= EDIT_MSG | VERIFY_MSG;
+		if (verbose_commit)
+			flags |= VERBOSE_COMMIT;
+	}
 	else if (is_fixup(command)) {
 		if (update_squash_messages(r, command, commit, opts))
 			return -1;
 		flags |= AMEND_MSG;
+		if (verbose_commit)
+			flags |= VERBOSE_COMMIT;
 		if (!final_fixup)
 			msg_file = rebase_path_squash_msg();
 		else if (file_exists(rebase_path_fixup_msg())) {
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 1723e1a858..9b410d31e2 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1477,4 +1477,60 @@ test_expect_success 'valid author header when author contains single quote' '
 	test_cmp expected actual
 '
 
+write_script "reword-and-check-for-diff" <<\EOF &&
+case "$1" in
+*/git-rebase-todo)
+	sed s/pick/reword/ "$1" > "$1.tmp"
+	mv -f "$1.tmp" "$1"
+	;;
+*)
+	grep '^diff --git' "$1" >has-diff
+	;;
+esac
+exit 0
+EOF
+
+test_expect_success 'rebase -i does not show diff by default when rewording' '
+	rebase_setup_and_clean no-verbose-commit-reword &&
+	test_set_editor "$PWD/reword-and-check-for-diff" &&
+	git rebase -i HEAD~1 &&
+	test_line_count = 0 has-diff
+'
+
+test_expect_success 'rebase -i respects rebase.verboseCommit when rewording' '
+	rebase_setup_and_clean verbose-commit-reword &&
+	test_config rebase.verboseCommit true &&
+	test_set_editor "$PWD/reword-and-check-for-diff" &&
+	git rebase -i HEAD~1 &&
+	test_line_count -gt 0 has-diff
+'
+
+write_script "squash-and-check-for-diff" <<\EOF &&
+case "$1" in
+*/git-rebase-todo)
+	sed "s/pick \([0-9a-f]*\) E/squash \1 E/" "$1" > "$1.tmp"
+	mv -f "$1.tmp" "$1"
+	;;
+*)
+	grep '^diff --git' "$1" >has-diff
+	;;
+esac
+exit 0
+EOF
+
+test_expect_success 'rebase -i does not show diff by default when squashing' '
+	rebase_setup_and_clean no-verbose-commit-squash &&
+	test_set_editor "$PWD/squash-and-check-for-diff" &&
+	git rebase -i HEAD~2 &&
+	test_line_count = 0 has-diff
+'
+
+test_expect_success 'rebase -i respects rebase.verboseCommit when squashing' '
+	rebase_setup_and_clean verbose-commit-squash &&
+	test_config rebase.verboseCommit true &&
+	test_set_editor "$PWD/squash-and-check-for-diff" &&
+	git rebase -i HEAD~2 &&
+	test_line_count -gt 0 has-diff
+'
+
 test_done

-- 
2.20.1
Ævar Arnfjörð Bjarmason· May 20, 2019, 23:49 UTC · re: Matheus Tavares · lore

Re: [GSoC] Some #leftoverbits for anyone looking for little projects

On Mon, May 20 2019, Matheus Tavares wrote:
Show 18 quoted lines
> Hi, Ævar
>
>> Give "rebase -i" some option so when you "reword" the patch is
>> included in the message.
>>
>> I keep going to the shell because I have no idea what change I'm
>> describing.
>
> I have the same problem, so I wanted to try solving this. The patch
> bellow creates a "rebase.verboseCommit" configuration that includes
> a diff when rewording or squashing. I'd appreciate knowing your thoughts
> on it.
>
> As Christian wisely pointed out to me, though, we can also achieve this
> behavior by setting "commit.verbose" to true. The only "downside" of it
> is that users cannot choose to see the diff only when rebasing. Despite
> of that, if we decide not to go with this patch, what do you think of
> adding a "commit.verbose" entry at git-rebase's man page?

Thanks for working on this. I'd somehow missed the addition of the commit.verbose option, so the problem I had is 100% solved by it (and I've turned it on).

I think it's better to just document it with rebase, perhaps rather than mention that option specifically (but that would also be fine) promise that we support "commit" options in general.

Do we promise anywhere that interactive rebase is going to run the "normal" git-commit command. From a quick skimming of the docs it doesn't seem so, perhaps we should explicitly promise that, and then test for it if we don't (e.g. by stealing the tests you added).

Aside from that, if this patch is kept I see commit.verbose is a bool-or-int option, but yours is maybe-bool, so there's no way with rebase.verboseCommit to turn on the higher level of verbosity. Perhaps if this option is kept some implementation that just grabs whatever "X" rebase.verboseCommit=X is set to and passes it as commit.verbase=X down to git-commit is better, letting it deal with the validation?

Show 146 quoted lines
> diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
> index d98e32d812..ae50b3e05d 100644
> --- a/Documentation/config/rebase.txt
> +++ b/Documentation/config/rebase.txt
> @@ -62,3 +62,8 @@ rebase.rescheduleFailedExec::
>  	Automatically reschedule `exec` commands that failed. This only makes
>  	sense in interactive mode (or when an `--exec` option was provided).
>  	This is the same as specifying the `--reschedule-failed-exec` option.
> +
> +rebase.verboseCommit::
> +	When rewording or squashing commits, during an interactive rebase, show
> +	the commits' diff to help describe the modifications they bring. False
> +	by default.
> diff --git a/sequencer.c b/sequencer.c
> index f88a97fb10..1596fc4cd0 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -914,6 +914,7 @@ N_("you have staged changes in your working tree\n"
>  #define CLEANUP_MSG (1<<3)
>  #define VERIFY_MSG  (1<<4)
>  #define CREATE_ROOT_COMMIT (1<<5)
> +#define VERBOSE_COMMIT (1<<6)
>
>  static int run_command_silent_on_success(struct child_process *cmd)
>  {
> @@ -1007,6 +1008,8 @@ static int run_git_commit(struct repository *r,
>  		argv_array_push(&cmd.args, "-n");
>  	if ((flags & AMEND_MSG))
>  		argv_array_push(&cmd.args, "--amend");
> +	if ((flags & VERBOSE_COMMIT))
> +		argv_array_push(&cmd.args, "-v");
>  	if (opts->gpg_sign)
>  		argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
>  	if (defmsg)
> @@ -1782,7 +1785,7 @@ static int do_pick_commit(struct repository *r,
>  	char *author = NULL;
>  	struct commit_message msg = { NULL, NULL, NULL, NULL };
>  	struct strbuf msgbuf = STRBUF_INIT;
> -	int res, unborn = 0, allow;
> +	int res, unborn = 0, allow, verbose_commit = 0;
>
>  	if (opts->no_commit) {
>  		/*
> @@ -1843,6 +1846,9 @@ static int do_pick_commit(struct repository *r,
>  		return error(_("cannot get commit message for %s"),
>  			oid_to_hex(&commit->object.oid));
>
> +	if (git_config_get_maybe_bool("rebase.verbosecommit", &verbose_commit) < 0)
> +		warning("Invalid value for rebase.verboseCommit. Using 'false' instead.");
> +
>  	if (opts->allow_ff && !is_fixup(command) &&
>  	    ((parent && oideq(&parent->object.oid, &head)) ||
>  	     (!parent && unborn))) {
> @@ -1853,6 +1859,8 @@ static int do_pick_commit(struct repository *r,
>  		if (res || command != TODO_REWORD)
>  			goto leave;
>  		flags |= EDIT_MSG | AMEND_MSG | VERIFY_MSG;
> +		if (verbose_commit)
> +			flags |= VERBOSE_COMMIT;
>  		msg_file = NULL;
>  		goto fast_forward_edit;
>  	}
> @@ -1909,12 +1917,17 @@ static int do_pick_commit(struct repository *r,
>  			author = get_author(msg.message);
>  	}
>
> -	if (command == TODO_REWORD)
> +	if (command == TODO_REWORD) {
>  		flags |= EDIT_MSG | VERIFY_MSG;
> +		if (verbose_commit)
> +			flags |= VERBOSE_COMMIT;
> +	}
>  	else if (is_fixup(command)) {
>  		if (update_squash_messages(r, command, commit, opts))
>  			return -1;
>  		flags |= AMEND_MSG;
> +		if (verbose_commit)
> +			flags |= VERBOSE_COMMIT;
>  		if (!final_fixup)
>  			msg_file = rebase_path_squash_msg();
>  		else if (file_exists(rebase_path_fixup_msg())) {
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 1723e1a858..9b410d31e2 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1477,4 +1477,60 @@ test_expect_success 'valid author header when author contains single quote' '
>  	test_cmp expected actual
>  '
>
> +write_script "reword-and-check-for-diff" <<\EOF &&
> +case "$1" in
> +*/git-rebase-todo)
> +	sed s/pick/reword/ "$1" > "$1.tmp"
> +	mv -f "$1.tmp" "$1"
> +	;;
> +*)
> +	grep '^diff --git' "$1" >has-diff
> +	;;
> +esac
> +exit 0
> +EOF
> +
> +test_expect_success 'rebase -i does not show diff by default when rewording' '
> +	rebase_setup_and_clean no-verbose-commit-reword &&
> +	test_set_editor "$PWD/reword-and-check-for-diff" &&
> +	git rebase -i HEAD~1 &&
> +	test_line_count = 0 has-diff
> +'
> +
> +test_expect_success 'rebase -i respects rebase.verboseCommit when rewording' '
> +	rebase_setup_and_clean verbose-commit-reword &&
> +	test_config rebase.verboseCommit true &&
> +	test_set_editor "$PWD/reword-and-check-for-diff" &&
> +	git rebase -i HEAD~1 &&
> +	test_line_count -gt 0 has-diff
> +'
> +
> +write_script "squash-and-check-for-diff" <<\EOF &&
> +case "$1" in
> +*/git-rebase-todo)
> +	sed "s/pick \([0-9a-f]*\) E/squash \1 E/" "$1" > "$1.tmp"
> +	mv -f "$1.tmp" "$1"
> +	;;
> +*)
> +	grep '^diff --git' "$1" >has-diff
> +	;;
> +esac
> +exit 0
> +EOF
> +
> +test_expect_success 'rebase -i does not show diff by default when squashing' '
> +	rebase_setup_and_clean no-verbose-commit-squash &&
> +	test_set_editor "$PWD/squash-and-check-for-diff" &&
> +	git rebase -i HEAD~2 &&
> +	test_line_count = 0 has-diff
> +'
> +
> +test_expect_success 'rebase -i respects rebase.verboseCommit when squashing' '
> +	rebase_setup_and_clean verbose-commit-squash &&
> +	test_config rebase.verboseCommit true &&
> +	test_set_editor "$PWD/squash-and-check-for-diff" &&
> +	git rebase -i HEAD~2 &&
> +	test_line_count -gt 0 has-diff
> +'
> +
>  test_done
Matheus Tavares Bernardino· May 21, 2019, 04:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [GSoC] Some #leftoverbits for anyone looking for little projects

On Mon, May 20, 2019 at 8:49 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 30 quoted lines
>
>
> On Mon, May 20 2019, Matheus Tavares wrote:
>
> > Hi, Ævar
> >
> >> Give "rebase -i" some option so when you "reword" the patch is
> >> included in the message.
> >>
> >> I keep going to the shell because I have no idea what change I'm
> >> describing.
> >
> > I have the same problem, so I wanted to try solving this. The patch
> > bellow creates a "rebase.verboseCommit" configuration that includes
> > a diff when rewording or squashing. I'd appreciate knowing your thoughts
> > on it.
> >
> > As Christian wisely pointed out to me, though, we can also achieve this
> > behavior by setting "commit.verbose" to true. The only "downside" of it
> > is that users cannot choose to see the diff only when rebasing. Despite
> > of that, if we decide not to go with this patch, what do you think of
> > adding a "commit.verbose" entry at git-rebase's man page?
>
> Thanks for working on this. I'd somehow missed the addition of the
> commit.verbose option, so the problem I had is 100% solved by it (and
> I've turned it on).
>
> I think it's better to just document it with rebase, perhaps rather than
> mention that option specifically (but that would also be fine) promise
> that we support "commit" options in general.
Indeed, it seems to be the right way to go.
> Do we promise anywhere that interactive rebase is going to run the
> "normal" git-commit command. From a quick skimming of the docs it
> doesn't seem so, perhaps we should explicitly promise that, and then
> test for it if we don't (e.g. by stealing the tests you added).

Ok, sounds good to me. In order to avoid duplicate tests, is it OK to assume that if one commit configuration is being respected by rebase, then all will be? Or should a patch adding such a promise include rebase tests for all commit.* configurations?

Johannes Schindelin· May 28, 2019, 10:37 UTC · re: Matheus Tavares · lore

Re: [GSoC] Some #leftoverbits for anyone looking for little projects

Hi Matheus,
On Mon, 20 May 2019, Matheus Tavares wrote:
Show 14 quoted lines
> > Give "rebase -i" some option so when you "reword" the patch is
> > included in the message.
> >
> > I keep going to the shell because I have no idea what change I'm
> > describing.
>
> I have the same problem, so I wanted to try solving this. The patch
> bellow creates a "rebase.verboseCommit" configuration that includes a
> diff when rewording or squashing. I'd appreciate knowing your thoughts
> on it.
>
> As Christian wisely pointed out to me, though, we can also achieve this
> behavior by setting "commit.verbose" to true. The only "downside" of it
> is that users cannot choose to see the diff only when rebasing.
You could of course add an alias like
	[alias]
		myrebase = -c commit.verbose=true rebase
which *should* work.

However, I am actually slightly in favor of your patch because it *does* make it more convenient to have this on during rebases only.

Ciao, Dscho

Matheus Tavares Bernardino· May 28, 2019, 17:37 UTC · re: Johannes Schindelin · lore

Re: [GSoC] Some #leftoverbits for anyone looking for little projects

On Tue, May 28, 2019 at 7:37 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 24 quoted lines
>
> Hi Matheus,
>
> On Mon, 20 May 2019, Matheus Tavares wrote:
>
> > > Give "rebase -i" some option so when you "reword" the patch is
> > > included in the message.
> > >
> > > I keep going to the shell because I have no idea what change I'm
> > > describing.
> >
> > I have the same problem, so I wanted to try solving this. The patch
> > bellow creates a "rebase.verboseCommit" configuration that includes a
> > diff when rewording or squashing. I'd appreciate knowing your thoughts
> > on it.
> >
> > As Christian wisely pointed out to me, though, we can also achieve this
> > behavior by setting "commit.verbose" to true. The only "downside" of it
> > is that users cannot choose to see the diff only when rebasing.
>
> You could of course add an alias like
>
>         [alias]
>                 myrebase = -c commit.verbose=true rebase
Hmm, I didn't know about `alias`. Thanks for the information.
> which *should* work.
>
> However, I am actually slightly in favor of your patch because it *does*
> make it more convenient to have this on during rebases only.

Another option we were discussing is to document that rebase obeys all commit.* options, instead of adding the rebase.verboseCommit config. Yes, this way we won't be able to toggle diff for rebase only, but I'm not sure if that's something users would want to do...

> Ciao,
> Dscho
Johannes Schindelin· May 28, 2019, 18:16 UTC · re: Matheus Tavares Bernardino · lore

Re: [GSoC] Some #leftoverbits for anyone looking for little projects

Hi Matheus,
On Tue, 28 May 2019, Matheus Tavares Bernardino wrote:
Show 36 quoted lines
> On Tue, May 28, 2019 at 7:37 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Mon, 20 May 2019, Matheus Tavares wrote:
> >
> > > > Give "rebase -i" some option so when you "reword" the patch is
> > > > included in the message.
> > > >
> > > > I keep going to the shell because I have no idea what change I'm
> > > > describing.
> > >
> > > I have the same problem, so I wanted to try solving this. The patch
> > > bellow creates a "rebase.verboseCommit" configuration that includes a
> > > diff when rewording or squashing. I'd appreciate knowing your thoughts
> > > on it.
> > >
> > > As Christian wisely pointed out to me, though, we can also achieve this
> > > behavior by setting "commit.verbose" to true. The only "downside" of it
> > > is that users cannot choose to see the diff only when rebasing.
> >
> > You could of course add an alias like
> >
> >         [alias]
> >                 myrebase = -c commit.verbose=true rebase
>
> Hmm, I didn't know about `alias`. Thanks for the information.
>
> > which *should* work.
> >
> > However, I am actually slightly in favor of your patch because it *does*
> > make it more convenient to have this on during rebases only.
>
> Another option we were discussing is to document that rebase obeys all
> commit.* options, instead of adding the rebase.verboseCommit config.
> Yes, this way we won't be able to toggle diff for rebase only, but I'm
> not sure if that's something users would want to do...

It is rather unintuitive that the `commit.*` options apply to a rebase. Sure, you could document it. But realistically, how many users will read it? Yes, I agree, that is a very low percentage.

Also: you yourself mentioned the rather convincing use case of `reword`.

Personally, I never really thought that I'd need `commit.verbose`. But your report made me think that I could use it *just* for `reword`, too.

So now you already have two active Git contributors wishing for that feature.

Ciao, Dscho

Johannes Schindelin· May 29, 2019, 09:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [GSoC] Some #leftoverbits for anyone looking for little projects

Hi Ævar,
On Sat, 17 Mar 2018, Ævar Arnfjörð Bjarmason wrote:
> In lieu of sending a PR to https://git.github.io/SoC-2018-Microprojects/
> I thought I'd list a few more suggestions, and hopefully others will
> chime in.

I am in the same camp, and figured that GitGitGadget (which *already* augments the Git mailing list-centric workflow via GitHub's convenient UI) would make for a fine location for these small left-over bits. So I added them to https://github.com/gitgitgadget/git/issues/234 (except the "git-unpack-*" idea, as I think that should be done as a test helper instead, and it should be done in the context of a new test case that actually needs this).

Ciao, Dscho

Johannes Schindelin· May 29, 2019, 09:40 UTC · re: Johannes Schindelin · lore

Re: [GSoC] Some #leftoverbits for anyone looking for little projects

Hi,
On Wed, 29 May 2019, Johannes Schindelin wrote:
Show 10 quoted lines
> On Sat, 17 Mar 2018, Ævar Arnfjörð Bjarmason wrote:
>
> > In lieu of sending a PR to https://git.github.io/SoC-2018-Microprojects/
> > I thought I'd list a few more suggestions, and hopefully others will
> > chime in.
>
> I am in the same camp, and figured that GitGitGadget (which *already*
> augments the Git mailing list-centric workflow via GitHub's convenient UI)
> would make for a fine location for these small left-over bits. So I added
> them to https://github.com/gitgitgadget/git/issues/234 (except the

Of course, I added them to https://github.com/gitgitgadget/git/issues/, with #234 being the last from this email.

Ciao, Dscho

Show 6 quoted lines
> "git-unpack-*" idea, as I think that should be done as a test helper
> instead, and it should be done in the context of a new test case that
> actually needs this).
>
> Ciao,
> Dscho
Jayati Shrivastava· Mar 20, 2022, 19:14 UTC · lore

Re: Having grep support the -o option

On Sat, Mar 19, 2022 at 9:49 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 14 quoted lines
>
>
> One thing I'd *really* like to see is the bits of %(if) %(then)
> etc. extracted from ref-filter.c into some general API other commands
> could use with strbuf_expand() and friends.
>
> I.e. if you could in addition to the strbuf_expand() callback define
> what verbs you support for "if" and the like, or have callbacks for
> their comparison functions.
>
> Then have that machinery drive the whole format expansion, which
> eventually would expand your %(some-custom-thing) via a callback.
>
> I.e. the whole "valid_atom" state machine in ref-filter.c.

So, the end goal is to design a formatting API that can be used by any command that takes --format option? Previous interns worked on unifying pretty.c and cat-file.c with ref-filter.c and so the next task can be to extend "valid_atom" state machine to work with more commands. Do you have any suggestions for new atoms/verbs I could add support for or any such similar small exercise to start with? +Hariom verma +Christian Couder +Git List

Christian Couder· Mar 22, 2022, 06:08 UTC · re: Jayati Shrivastava · lore

Re: Having grep support the -o option

On Sun, Mar 20, 2022 at 8:14 PM Jayati Shrivastava <gaurijove@gmail.com> wrote:
Show 20 quoted lines
>
> On Sat, Mar 19, 2022 at 9:49 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> >
> >
> > One thing I'd *really* like to see is the bits of %(if) %(then)
> > etc. extracted from ref-filter.c into some general API other commands
> > could use with strbuf_expand() and friends.
> >
> > I.e. if you could in addition to the strbuf_expand() callback define
> > what verbs you support for "if" and the like, or have callbacks for
> > their comparison functions.
> >
> > Then have that machinery drive the whole format expansion, which
> > eventually would expand your %(some-custom-thing) via a callback.
> >
> > I.e. the whole "valid_atom" state machine in ref-filter.c.
>
> So, the end goal is to design a formatting API that can be used by any
> command that takes --format option?

It might be nice if we get closer to this after your GSoC project, but I don't think it should become the main goal of the GSoC.

> Previous interns worked on
> unifying pretty.c and cat-file.c with ref-filter.c and so the next task can
> be to extend "valid_atom" state machine to work with more commands.

Note that performance issues have been a really big issue in unifying cat-file.c with ref-filter.c.

> Do you have any suggestions for new atoms/verbs I could add
> support for or any such similar small exercise to start with?

It would be nice if we first got an idea of the features in pretty.c that do not yet have something similar in ref-filter.c. I think Hariom used to show how features in pretty.c corresponded to features in ref-filter.c, and this helped with deciding about the next steps to take.

Ævar Arnfjörð Bjarmason· Mar 22, 2022, 10:50 UTC · re: Christian Couder · lore

Re: Having grep support the -o option

On Tue, Mar 22 2022, Christian Couder wrote:
Show 24 quoted lines
> On Sun, Mar 20, 2022 at 8:14 PM Jayati Shrivastava <gaurijove@gmail.com> wrote:
>>
>> On Sat, Mar 19, 2022 at 9:49 PM Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>> >
>> >
>> > One thing I'd *really* like to see is the bits of %(if) %(then)
>> > etc. extracted from ref-filter.c into some general API other commands
>> > could use with strbuf_expand() and friends.
>> >
>> > I.e. if you could in addition to the strbuf_expand() callback define
>> > what verbs you support for "if" and the like, or have callbacks for
>> > their comparison functions.
>> >
>> > Then have that machinery drive the whole format expansion, which
>> > eventually would expand your %(some-custom-thing) via a callback.
>> >
>> > I.e. the whole "valid_atom" state machine in ref-filter.c.
>>
>> So, the end goal is to design a formatting API that can be used by any
>> command that takes --format option?
>
> It might be nice if we get closer to this after your GSoC project, but
> I don't think it should become the main goal of the GSoC.

Agreed. FWIW this is off-list discussion between Jayati and myself which started with her asking (and this was omitted in the context that made it on list:

    [...] Infact, I'll be excited to work on anything you suggest even
    if its not related to the project, as it will help me get familiar
    with the codebase and the contribution process at Git.

So that suggestion of mine of generalizing ref-filter.c wasn't meant to distract from the GSoC project, I understood her to be looking for suggestions for things to work on that were *not* part of the GSoC project.

So I suggested that ref-filter.c task, which is orthagonal, but relates to some of the same code.

Jayati Shrivastava· Mar 23, 2022, 17:45 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: Having grep support the -o option

On Tue, Mar 22, 2022 at 4:24 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 44 quoted lines
>
>
> On Tue, Mar 22 2022, Christian Couder wrote:
>
> > On Sun, Mar 20, 2022 at 8:14 PM Jayati Shrivastava <gaurijove@gmail.com> wrote:
> >>
> >> On Sat, Mar 19, 2022 at 9:49 PM Ævar Arnfjörð Bjarmason
> >> <avarab@gmail.com> wrote:
> >> >
> >> >
> >> > One thing I'd *really* like to see is the bits of %(if) %(then)
> >> > etc. extracted from ref-filter.c into some general API other commands
> >> > could use with strbuf_expand() and friends.
> >> >
> >> > I.e. if you could in addition to the strbuf_expand() callback define
> >> > what verbs you support for "if" and the like, or have callbacks for
> >> > their comparison functions.
> >> >
> >> > Then have that machinery drive the whole format expansion, which
> >> > eventually would expand your %(some-custom-thing) via a callback.
> >> >
> >> > I.e. the whole "valid_atom" state machine in ref-filter.c.
> >>
> >> So, the end goal is to design a formatting API that can be used by any
> >> command that takes --format option?
> >
> > It might be nice if we get closer to this after your GSoC project, but
> > I don't think it should become the main goal of the GSoC.
>
> Agreed. FWIW this is off-list discussion between Jayati and myself which
> started with her asking (and this was omitted in the context that made
> it on list:
>
>     [...] Infact, I'll be excited to work on anything you suggest even
>     if its not related to the project, as it will help me get familiar
>     with the codebase and the contribution process at Git.
>
> So that suggestion of mine of generalizing ref-filter.c wasn't meant to
> distract from the GSoC project, I understood her to be looking for
> suggestions for things to work on that were *not* part of the GSoC
> project.
>
> So I suggested that ref-filter.c task, which is orthagonal, but relates
> to some of the same code.

I understand this might be out of scope for the gsoc project but I’ll try to work in the general direction of it. I have been going through Hariom’s work from last year. He has mentioned some failing log tests for the new pretty library he implemented. I am currently attempting to understand these failures.

← back to recent threads