Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-09 Thread Johannes Schindelin
Hi,

On 2015-06-08 23:00, Michael Rappazzo wrote:

 diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
 index dc3133f..b92375e 100644
 --- a/git-rebase--interactive.sh
 +++ b/git-rebase--interactive.sh
 @@ -977,7 +977,9 @@ else
   revisions=$onto...$orig_head
   shortrevisions=$shorthead
  fi
 -git rev-list $merges_option --pretty=oneline --reverse --left-right
 --topo-order \
 +format=$(git config --get rebase.instructionFormat)
 +# the 'rev-list .. | sed' requires %m to parse; the instruction
 requires %H to parse
 +git rev-list $merges_option --format=%m%H ${format-%s} --reverse
 --left-right --topo-order \

These two lines are too long (longer than 80 columns)...

Besides, are you sure you don't want to substitute an empty 
'rebase.instructionFormat' by '%s'? I would have expected to read 
`${format:-%s}` (note the colon), but then, this was Junio's suggestion... 
Junio, what do you think, should we not rather substitute empty values by `%s` 
as if the config setting was unset?

   $revisions ${restrict_revision+^$restrict_revision} | \
   sed -n s/^//p |
  while read -r sha1 rest

Ciao,
Johannes
--
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: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-09 Thread Mike Rappazzo
I see your point, and I'll explore that avenue.

Personally, I like the idea that one could also use the short hash if
the custom instruction started with %h , but I see the value in
leaving the variable blank.

After running the tests with a custom format enabled, I did find that
autosquash doesn't work, so I am working to correct that.

On Tue, Jun 9, 2015 at 5:36 AM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 Hi,

 On 2015-06-08 23:00, Michael Rappazzo wrote:

 diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
 index dc3133f..b92375e 100644
 --- a/git-rebase--interactive.sh
 +++ b/git-rebase--interactive.sh
 @@ -977,7 +977,9 @@ else
   revisions=$onto...$orig_head
   shortrevisions=$shorthead
  fi
 -git rev-list $merges_option --pretty=oneline --reverse --left-right
 --topo-order \
 +format=$(git config --get rebase.instructionFormat)
 +# the 'rev-list .. | sed' requires %m to parse; the instruction
 requires %H to parse
 +git rev-list $merges_option --format=%m%H ${format-%s} --reverse
 --left-right --topo-order \

 These two lines are too long (longer than 80 columns)...

 Besides, are you sure you don't want to substitute an empty 
 'rebase.instructionFormat' by '%s'? I would have expected to read 
 `${format:-%s}` (note the colon), but then, this was Junio's suggestion... 
 Junio, what do you think, should we not rather substitute empty values by 
 `%s` as if the config setting was unset?

   $revisions ${restrict_revision+^$restrict_revision} | \
   sed -n s/^//p |
  while read -r sha1 rest

 Ciao,
 Johannes
--
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: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-09 Thread Junio C Hamano
Johannes Schindelin johannes.schinde...@gmx.de writes:

 Besides, are you sure you don't want to substitute an empty
 rebase.instructionFormat' by '%s'? I would have expected to read
 ${format:-%s}` (note the colon), but then, this was Junio's
 suggestion...

That was me simply being sloppy myself, expecting people not to copy
and paste literally without thinking.  Thanks for noticing.
--
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: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-09 Thread Mike Rappazzo
I have since reworked this script to support the short hash in the
custom format as a special case:

-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+format=$(git config --get rebase.instructionFormat)
+no_format=$?
+if test ${no_format} -ne 0
+then
+ format=%H %s
+elif test ${format:0:3} != %H   test ${format:0:3} != %h 
+then
+ format=%H ${format}
+fi
+# the 'rev-list .. | sed' requires %m to parse; the instruction
requires %H to parse
+git rev-list $merges_option --format=%m${format} \
+ --reverse --left-right --topo-order \

I also use the $no_format variable later on in the autosquash
re-ordering, and have the tests passing.  I want to add some new tests
on the custom format, and will send a new patch when that is complete.

On Tue, Jun 9, 2015 at 3:23 PM, Junio C Hamano gits...@pobox.com wrote:
 Johannes Schindelin johannes.schinde...@gmx.de writes:

 Besides, are you sure you don't want to substitute an empty
 rebase.instructionFormat' by '%s'? I would have expected to read
 ${format:-%s}` (note the colon), but then, this was Junio's
 suggestion...

 That was me simply being sloppy myself, expecting people not to copy
 and paste literally without thinking.  Thanks for noticing.
--
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: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format

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

 I have since reworked this script to support the short hash in the
 custom format as a special case:

I thought that we always give short form when presenting it to the
end user to edit, but for internal bookkeeping purposes we make sure
that we use the full SHA-1, so that new objects created during the
rebase session will not cause used to be unique but not anymore
ambiguity in the commit object names in the instruction sheet.

I have been assuming that the rev-list we have been mucking with
was to prepare the latter, the internal bookkeeping data, which must
always spell 40-hex (that is why the default oneline is not
--oneline but --pretty=oneline).

Why is it necessary to make %m%H part customizable in the first
place?

Puzzled


 -git rev-list $merges_option --pretty=oneline --reverse --left-right
 --topo-order \
 +format=$(git config --get rebase.instructionFormat)
 +no_format=$?
 +if test ${no_format} -ne 0
 +then
 + format=%H %s
 +elif test ${format:0:3} != %H   test ${format:0:3} != %h 

Do not use bash-ism substring.

 +then
 + format=%H ${format}
 +fi
 +# the 'rev-list .. | sed' requires %m to parse; the instruction
 requires %H to parse
 +git rev-list $merges_option --format=%m${format} \
 + --reverse --left-right --topo-order \

 I also use the $no_format variable later on in the autosquash
 re-ordering, and have the tests passing.  I want to add some new tests
 on the custom format, and will send a new patch when that is complete.

 On Tue, Jun 9, 2015 at 3:23 PM, Junio C Hamano gits...@pobox.com wrote:
 Johannes Schindelin johannes.schinde...@gmx.de writes:

 Besides, are you sure you don't want to substitute an empty
 rebase.instructionFormat' by '%s'? I would have expected to read
 ${format:-%s}` (note the colon), but then, this was Junio's
 suggestion...

 That was me simply being sloppy myself, expecting people not to copy
 and paste literally without thinking.  Thanks for noticing.
--
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: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-08 Thread Junio C Hamano
Michael Rappazzo rappa...@gmail.com writes:

 A config option 'rebase.instructionFormat' can override the
 default 'oneline' format of the rebase instruction list.

 Since the list is parsed using the left, right or boundary mark plus
 the sha1, they are prepended to the instruction format.

 Signed-off-by: Michael Rappazzo rappa...@gmail.com
 ---
  git-rebase--interactive.sh | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
 index dc3133f..b92375e 100644
 --- a/git-rebase--interactive.sh
 +++ b/git-rebase--interactive.sh
 @@ -977,7 +977,9 @@ else
   revisions=$onto...$orig_head
   shortrevisions=$shorthead
  fi
 -git rev-list $merges_option --pretty=oneline --reverse --left-right 
 --topo-order \
 +format=$(git config --get rebase.instructionFormat)
 +# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H 
 to parse
 +git rev-list $merges_option --format=%m%H ${format-%s} --reverse 
 --left-right --topo-order \
   $revisions ${restrict_revision+^$restrict_revision} | \
   sed -n s/^//p |
  while read -r sha1 rest

Looks OK, but

 - Needs a patch to Documentation/config.txt and possibly also
   Documentation/git-rebase.txt

 - Needs tests somewhere in t34?? series.

Also I think git rev-list line has got way too long.  As it is
already using backslash-continuation, do not hesitate to fold it
further, e.g. 

git rev-list $merges_option --format=%m%H ${format-%s} \
--reverse --left-right --topo-order \
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n s/^//p |
while ...

Thanks.
--
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


[PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-08 Thread Michael Rappazzo
Difference between v1 and v2 of this patch:

- Fixed indentation from spaces to match the existing style
- Changed the prepended sha1 from short (%h) to long (%H)
- Used bash variable default when the config option is not present

Michael Rappazzo (1):
  git-rebase--interactive.sh: add config option for custom instruction
format

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

-- 
2.4.2

--
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


[PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-08 Thread Michael Rappazzo
A config option 'rebase.instructionFormat' can override the
default 'oneline' format of the rebase instruction list.

Since the list is parsed using the left, right or boundary mark plus
the sha1, they are prepended to the instruction format.

Signed-off-by: Michael Rappazzo rappa...@gmail.com
---
 git-rebase--interactive.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..b92375e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -977,7 +977,9 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
 fi
-git rev-list $merges_option --pretty=oneline --reverse --left-right 
--topo-order \
+format=$(git config --get rebase.instructionFormat)
+# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to 
parse
+git rev-list $merges_option --format=%m%H ${format-%s} --reverse 
--left-right --topo-order \
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n s/^//p |
 while read -r sha1 rest
-- 
2.4.2

--
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: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-08 Thread Junio C Hamano
Michael Rappazzo rappa...@gmail.com writes:

 Difference between v1 and v2 of this patch:

 - Fixed indentation from spaces to match the existing style
 - Changed the prepended sha1 from short (%h) to long (%H)
 - Used bash variable default when the config option is not present

 Michael Rappazzo (1):
   git-rebase--interactive.sh: add config option for custom instruction
 format

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

Does this pass tests?  I see many failures including t3415.
--
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