Re: Suggestion: add author info to TODO list in git-rebase--interactive

2015-06-07 Thread Junio C Hamano
Mike Rappazzo rappa...@gmail.com writes:

 diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
 index dc3133f..e2d5ffc 100644
 --- a/git-rebase--interactive.sh
 +++ b/git-rebase--interactive.sh
 @@ -977,7 +977,18 @@ else
   revisions=$onto...$orig_head
   shortrevisions=$shorthead
  fi
 -git rev-list $merges_option --pretty=oneline --reverse --left-right
 --topo-order \
 +custom_format=$(git config --get rebase.interactive.todo-format)

We use three-level names only when we need an unbounded end-user
supplied things (e.g. nickname for remotes in remote.*.url,
description for a branch in branch.*.description).  interactive
is not such a token, so a two-level name, e.g. rebase.insnformat
or something like that.  Also core Git avoids variable names with
- in them.

 +if test -z $custom_format
 +then
 +   custom_format=oneline
 +else
 +   # the custom format MUST start with %m%h or %m%H
 + if test ${custom_format:0:5} != '%m%h '
 +   then
 +  custom_format=%m%h ${custom_format}
 +   fi
 +fi

Why not allow them to *ONLY* set what follows '%m%h '?  That is, if
they say '%m%h %s', give them '%m%h %m%h %s', by unconditionally
prepend '%m%h '.  That way you do not need these conditional.

Something along the lines of...

format=$(git config rebase.insnFormat)
if test -z $format
then
format=%m%h%s
else
format=%m%h$format
fi

git rev-list $merge_option --format=$format --reverse \
--topo-order --left-right \
...

 ...  I also tried changing the
 '--left-right' to '--left-only', but that seemed to not produce any
 results.

Wouldn't we want right side of the symmetric difference, though?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Suggestion: add author info to TODO list in git-rebase--interactive

2015-06-05 Thread Mike Rappazzo
I find that If I am doing a rebase with the intention to squash or
re-order commits, it is helpful to know the commit author.

However, the alteration that I have made to git-rebase--interactive
may not be entirely correct.  Here is the change:

---
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..ec44d41 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -977,7 +977,7 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
 fi
-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+git rev-list $merges_option --pretty=%m%h [%an] %x09%s --reverse
--left-right --topo-order \
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n s/^//p |
 while read -r sha1 rest
--
2.4.2


The problem, as I see it is that the original '--pretty=oneline' only
produces a single line of output (of course).  However, the changed
version '--pretty=%m%h [%an] %x09%s' produces multiple lines.  The
command seems to ignore the unimportant lines, and the expected output
is put into the TODO list though.

Is there a better way of providing this information, or is this still
acceptable?

_Mike
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Suggestion: add author info to TODO list in git-rebase--interactive

2015-06-05 Thread Eric Sunshine
On Fri, Jun 5, 2015 at 3:35 PM, Junio C Hamano gits...@pobox.com wrote:
 Mike Rappazzo rappa...@gmail.com writes:
 I find that If I am doing a rebase with the intention to squash or
 re-order commits, it is helpful to know the commit author.

 There is not a fundamental reason why the remainder of the line
 after the object name in the rebase insn sheet should not be
 customizable, and I think your patch is a good first step to
 identify where that customization should go.

 But that is a customization issue, not changing the default and the
 only format used.

The idea of being able to provide a custom format for insn sheet lines
came up within the last year and was somewhat more well-developed and
a bit more heavily discussed. I don't recall whether there was an
accompanying patch, and I am unfortunately unable to locate the
discussion in the archive.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Suggestion: add author info to TODO list in git-rebase--interactive

2015-06-05 Thread Junio C Hamano
Mike Rappazzo rappa...@gmail.com writes:

 I find that If I am doing a rebase with the intention to squash or
 re-order commits, it is helpful to know the commit author.

There is not a fundamental reason why the remainder of the line
after the object name in the rebase insn sheet should not be
customizable, and I think your patch is a good first step to
identify where that customization should go.

But that is a customization issue, not changing the default and the
only format used.

 diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
 index dc3133f..ec44d41 100644
 --- a/git-rebase--interactive.sh
 +++ b/git-rebase--interactive.sh
 @@ -977,7 +977,7 @@ else
 revisions=$onto...$orig_head
 shortrevisions=$shorthead
  fi
 -git rev-list $merges_option --pretty=oneline --reverse --left-right
 --topo-order \
 +git rev-list $merges_option --pretty=%m%h [%an] %x09%s --reverse
 --left-right --topo-order \
 $revisions ${restrict_revision+^$restrict_revision} | \
 sed -n s/^//p |
  while read -r sha1 rest

This is nothing new, and it may not even be a problem, but why do we
use --left-right and then filter with sed?  Does this part of the
code predate --left-only, I wonder.  I'd probably write

git log --format=%h%s --left-only .

with today's Git, but perhaps I am missing something?

 The problem, as I see it is that the original '--pretty=oneline' only
 produces a single line of output (of course).  However, the changed
 version '--pretty=%m%h [%an] %x09%s' produces multiple lines.

Shouldn't you be using tformat, not format, if you are doing a
oneline emulation?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Suggestion: add author info to TODO list in git-rebase--interactive

2015-06-05 Thread Mike Rappazzo
I think the custom format makes sense.  I took a first pass.

A config option 'rebase.interactive.todo-format' can override the
default 'oneline' format of the TODO list.  Since the list is
parsed using the left, right or boundary mark plus the sha1, then if the
custom format does not start with those values, they will be
automatically added to the beginning of the custom format.

For example, if author information is desired for the TODO list, then
setting the config value to '[%an] %s' will actually result in the
format being '%m%h [%an] %s'

---
 git-rebase--interactive.sh | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..e2d5ffc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -977,7 +977,18 @@ else
  revisions=$onto...$orig_head
  shortrevisions=$shorthead
 fi
-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+custom_format=$(git config --get rebase.interactive.todo-format)
+if test -z $custom_format
+then
+   custom_format=oneline
+else
+   # the custom format MUST start with %m%h or %m%H
+ if test ${custom_format:0:5} != '%m%h '
+   then
+  custom_format=%m%h ${custom_format}
+   fi
+fi
+git rev-list $merges_option --pretty=${custom_format} --reverse
--left-right --topo-order \
  $revisions ${restrict_revision+^$restrict_revision} | \
  sed -n s/^//p |
 while read -r sha1 rest
-- 

Is this closer to what you are looking for?  I also tried changing the
'--left-right' to '--left-only', but that seemed to not produce any
results.




On Fri, Jun 5, 2015 at 3:39 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Fri, Jun 5, 2015 at 3:35 PM, Junio C Hamano gits...@pobox.com wrote:
 Mike Rappazzo rappa...@gmail.com writes:
 I find that If I am doing a rebase with the intention to squash or
 re-order commits, it is helpful to know the commit author.

 There is not a fundamental reason why the remainder of the line
 after the object name in the rebase insn sheet should not be
 customizable, and I think your patch is a good first step to
 identify where that customization should go.

 But that is a customization issue, not changing the default and the
 only format used.

 The idea of being able to provide a custom format for insn sheet lines
 came up within the last year and was somewhat more well-developed and
 a bit more heavily discussed. I don't recall whether there was an
 accompanying patch, and I am unfortunately unable to locate the
 discussion in the archive.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html