Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Junio C Hamano]

Junio C Hamano  writes:

> Jeff King  writes:
>
>> On Mon, Feb 06, 2017 at 02:34:08PM -0800, Junio C Hamano wrote:
>>
>>> * sk/parse-remote-cleanup (2017-02-06) 1 commit
>>>   (merged to 'next' on 2017-02-06 at 6ec89f72d5)
>>>  + parse-remote: remove reference to unused op_prep
>>> 
>>>  Code clean-up.
>>> 
>>>  Will merge to 'master'.
>>
>> Hrm. Are the functions in git-parse-remote.sh part of the public API?
>> That is, do we expect third-party scripts to do:
>>
>>   . "$(git rev-parse --exec)/git-parse-remote.sh
>>   error_on_missing_default_upstream "$a" "$b" "$c" "$d"
>>
>> ? If so, then they may be surprised by the change in function signature.
>>
>> I generally think of git-sh-setup as the one that external scripts would
>> use. There _is_ a manpage for git-parse-remote, but it doesn't list any
>> functions. So maybe they're all fair game for changing?
>>
>> I just didn't see any discussion of this in the original patch thread,
>> so I wanted to make sure we were making that decision consciously, and
>> not accidentally. :)
>
> Ummm, yes, I admit that this was accidental.  I didn't really think
> of parse-remote as an externally visible and supported interface,
> but users have tendency to break our expectations, so, I dunno.

After sleeping on this, I doubt that the value of this "code
clean-up" is worth the trouble of waiting to see if a third-party
who dot sources parse-remote steps up, which may never materialize
while the topic is cooking in 'next' and more importantly risking
breakage on such a third-party.

Let's drop the topic and excise it from 'next' at the next version
boundary.

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Johannes Schindelin]

Hi Junio,


On Thu, 9 Feb 2017, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > ... and even go so far as calling your patch reverting my refactoring
> > a hot-fix, why don't you just go ahead and merge the result over my
> > objections?
> 
> At this point, you are simply being silly.

How is it that this patch cannot be applied when, and if, that
hypothetical config setting is introduced?

Maybe I am dense here, but I would really like to know why this
preparatory patch must be applied *now*, when there is nothing to prepare
for.

> Isn't "Putty is not a command but is also handled as if it is a valid
> implementation of SSH" a bug?

If you think it is a bug to handle an ssh command called "putty" as if it
were plink, sure. I do not think there is a valid use case, but hey.

Ciao,
Johannes

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Junio C Hamano]

Johannes Schindelin  writes:

>> > (And that would have to be handled at a different point, as I had
>> > pointed out, so that suggested preparation would most likely not help
>> > at all.)
>> 
>> I did not think "that would have to be handled at a different point"
>> is correct at all, if by "a point" you meant "a location in the
>> code" [*1*].
>
> Yes, I mean the location in the code.
>
> But since you keep insisting that you are right and I am wrong,...

There is no "insisting".  Didn't we just see how wrong you were
about "different point"?  An extended syntax of override would be
handled in the new helper to handle override, the same point in the
code as other overrides are handled.

> ... and even
> go so far as calling your patch reverting my refactoring a hot-fix, why
> don't you just go ahead and merge the result over my objections?

At this point, you are simply being silly.

Isn't "Putty is not a command but is also handled as if it is a
valid implementation of SSH" a bug?  Isn't making the code not to be
confused like so a fix?

A different approach to fix the issue would be to declare that the
command names and overrides are not in separate namespaces.

If you prefer to go that route, the documentation can use an update
to make it not mention "putty" as a valid override (the users have
to spell "plink"), mention "plink.exe" is also accepted, etc. and
make it clear that the override environment and configuration
variables are the way to tell Git: "The ssh implementation I have
behaves the same way as this well-known implementation, so treat it
as such without actually looking at the path to the command in the
ssh.command string".

That may limit the freedom for future enhancement of overrides, but
is an acceptable short-cut.  After all, the overrides are merely an
escape hatch and we expect them to be used only rarely.

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Johannes Schindelin]

Hi Junio,

On Thu, 9 Feb 2017, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > (And that would have to be handled at a different point, as I had
> > pointed out, so that suggested preparation would most likely not help
> > at all.)
> 
> I did not think "that would have to be handled at a different point"
> is correct at all, if by "a point" you meant "a location in the
> code" [*1*].

Yes, I mean the location in the code.

But since you keep insisting that you are right and I am wrong, and even
go so far as calling your patch reverting my refactoring a hot-fix, why
don't you just go ahead and merge the result over my objections?

Ciao,
Johannes

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Junio C Hamano]

Michael Haggerty  writes:

> On 02/06/2017 11:34 PM, Junio C Hamano wrote:
>> [...]
>> --
>> [Stalled]
>> [...]
>> * mh/ref-remove-empty-directory (2017-01-07) 23 commits
>>  - files_transaction_commit(): clean up empty directories
>>  - try_remove_empty_parents(): teach to remove parents of reflogs, too
>>  - try_remove_empty_parents(): don't trash argument contents
>>  - try_remove_empty_parents(): rename parameter "name" -> "refname"
>>  - delete_ref_loose(): inline function
>>  - delete_ref_loose(): derive loose reference path from lock
>>  - log_ref_write_1(): inline function
>>  - log_ref_setup(): manage the name of the reflog file internally
>>  - log_ref_write_1(): don't depend on logfile argument
>>  - log_ref_setup(): pass the open file descriptor back to the caller
>>  - log_ref_setup(): improve robustness against races
>>  - log_ref_setup(): separate code for create vs non-create
>>  - log_ref_write(): inline function
>>  - rename_tmp_log(): improve error reporting
>>  - rename_tmp_log(): use raceproof_create_file()
>>  - lock_ref_sha1_basic(): use raceproof_create_file()
>>  - lock_ref_sha1_basic(): inline constant
>>  - raceproof_create_file(): new function
>>  - safe_create_leading_directories(): set errno on SCLD_EXISTS
>>  - safe_create_leading_directories_const(): preserve errno
>>  - t5505: use "for-each-ref" to test for the non-existence of references
>>  - refname_is_safe(): correct docstring
>>  - files_rename_ref(): tidy up whitespace
>> 
>>  Deletion of a branch "foo/bar" could remove .git/refs/heads/foo
>>  once there no longer is any other branch whose name begins with
>>  "foo/", but we didn't do so so far.  Now we do.
>> 
>>  Expecting a reroll.
>>  cf. <5051c78e-51f9-becd-e1a6-9c0b781d6...@alum.mit.edu>
>
> I think you missed v4 of this patch series [1], which is the re-roll
> that you were waiting for. And I missed that you missed it...
>
> Michael
>
> [1] http://public-inbox.org/git/cover.1483719289.git.mhag...@alum.mit.edu/

Actually it was worse than that.  What the above lists *is* v4; I
just failed to update "Expecting a reroll" note when I updated the
topic with your rerolled patches, and left it there trusting the
now-stale note of mine.

Sorry, and a HUGE thanks for noticing the mistake.

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Junio C Hamano]

Michael Haggerty  writes:

> On 02/06/2017 11:34 PM, Junio C Hamano wrote:
>> [...]
>> --
>> [Stalled]
>> [...]
>> * mh/ref-remove-empty-directory (2017-01-07) 23 commits
>> 
>>  Deletion of a branch "foo/bar" could remove .git/refs/heads/foo
>>  once there no longer is any other branch whose name begins with
>>  "foo/", but we didn't do so so far.  Now we do.
>> 
>>  Expecting a reroll.
>>  cf. <5051c78e-51f9-becd-e1a6-9c0b781d6...@alum.mit.edu>
>
> I think you missed v4 of this patch series [1], which is the re-roll
> that you were waiting for. And I missed that you missed it...
>
> Michael
>
> [1] http://public-inbox.org/git/cover.1483719289.git.mhag...@alum.mit.edu/

Great.  Thanks.

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Junio C Hamano]

Johannes Schindelin  writes:

> (And that would have to be handled at a different point, as I
> had pointed out, so that suggested preparation would most likely not help
> at all.)

I did not think "that would have to be handled at a different point"
is correct at all, if by "a point" you meant "a location in the
code" [*1*].  If we want to make it configurable in a more detailed
way by directly allowing to override port_option and needs_batch
separately, you would do that in override_ssh_variant(), without
touching handle_ssh_variant() in the refactored code.  That way, you
do not have to worry about breaking the auto detection based on the
command name.


[Footnote]

*1* Or did you mean "point in time", aka "let's do it outside the
scope of this patch series"?

Let's not keep it as a SQUASH??? proposal, but as a separate hot-fix
follow-up patch.

-- >8 --
Subject: connect.c: stop conflating ssh command names and overrides

dd33e07766 ("connect: Add the envvar GIT_SSH_VARIANT and ssh.variant
config", 2017-02-01) attempted to add support for configuration and
environment variable to override the different handling of
port_option and needs_batch settings suitable for variants of the
ssh implementation that was autodetected by looking at the ssh
command name.  Because it piggybacked on the code that turns command
name to specific override (e.g. "plink.exe" and "plink" means
port_option needs to be set to 'P' instead of the default 'p'), yet
it defined a separate namespace for these overrides (e.g. "putty"
can be usable to signal that port_option needs to be 'P'), however,
it made the auto-detection based on the command name less robust
(e.g. the code now accepts "putty" as a SSH command name and applies
the same override).

Separate the code that interprets the override that was read from
the configuration & environment from the original code that handles
the command names, as they are in separate namespaces, to fix this
confusion.

This incidentally also makes it easier for future enhancement of the
override syntax (e.g. "port_option=p,needs_batch=1" may want to be
accepted as a more explicit syntax) without affecting the code for
auto-detection based on the command name.

While at it, update the return type of the handle_ssh_variant()
helper function to void; the caller does not use it, and the
function does not return any meaningful value.

Signed-off-by: Junio C Hamano 
---
 connect.c | 45 -
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/connect.c b/connect.c
index 7f1f802396..7d65c1c736 100644
--- a/connect.c
+++ b/connect.c
@@ -691,17 +691,39 @@ static const char *get_ssh_command(void)
return NULL;
 }
 
-static int handle_ssh_variant(const char *ssh_command, int is_cmdline,
- int *port_option, int *needs_batch)
+static int override_ssh_variant(int *port_option, int *needs_batch)
 {
-   const char *variant = getenv("GIT_SSH_VARIANT");
+   char *variant;
+
+   variant = xstrdup_or_null(getenv("GIT_SSH_VARIANT"));
+   if (!variant &&
+   git_config_get_string("ssh.variant", ))
+   return 0;
+
+   if (!strcmp(variant, "plink") || !strcmp(variant, "putty")) {
+   *port_option = 'P';
+   *needs_batch = 0;
+   } else if (!strcmp(variant, "tortoiseplink")) {
+   *port_option = 'P';
+   *needs_batch = 1;
+   } else {
+   *port_option = 'p';
+   *needs_batch = 0;
+   }
+   free(variant);
+   return 1;
+}
+
+static void handle_ssh_variant(const char *ssh_command, int is_cmdline,
+  int *port_option, int *needs_batch)
+{
+   const char *variant;
char *p = NULL;
 
-   if (variant)
-   ; /* okay, fall through */
-   else if (!git_config_get_string("ssh.variant", ))
-   variant = p;
-   else if (!is_cmdline) {
+   if (override_ssh_variant(port_option, needs_batch))
+   return;
+
+   if (!is_cmdline) {
p = xstrdup(ssh_command);
variant = basename(p);
} else {
@@ -717,12 +739,11 @@ static int handle_ssh_variant(const char *ssh_command, 
int is_cmdline,
 */
free(ssh_argv);
} else
-   return 0;
+   return;
}
 
if (!strcasecmp(variant, "plink") ||
-   !strcasecmp(variant, "plink.exe") ||
-   !strcasecmp(variant, "putty"))
+   !strcasecmp(variant, "plink.exe"))
*port_option = 'P';
else if (!strcasecmp(variant, "tortoiseplink") ||
 !strcasecmp(variant, "tortoiseplink.exe")) {
@@ -730,8 +751,6 @@ static int handle_ssh_variant(const char *ssh_command, int 
is_cmdline,
*needs_batch = 1;
}
free(p);
-
- 

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Michael Haggerty]

On 02/06/2017 11:34 PM, Junio C Hamano wrote:
> [...]
> --
> [Stalled]
> [...]
> * mh/ref-remove-empty-directory (2017-01-07) 23 commits
>  - files_transaction_commit(): clean up empty directories
>  - try_remove_empty_parents(): teach to remove parents of reflogs, too
>  - try_remove_empty_parents(): don't trash argument contents
>  - try_remove_empty_parents(): rename parameter "name" -> "refname"
>  - delete_ref_loose(): inline function
>  - delete_ref_loose(): derive loose reference path from lock
>  - log_ref_write_1(): inline function
>  - log_ref_setup(): manage the name of the reflog file internally
>  - log_ref_write_1(): don't depend on logfile argument
>  - log_ref_setup(): pass the open file descriptor back to the caller
>  - log_ref_setup(): improve robustness against races
>  - log_ref_setup(): separate code for create vs non-create
>  - log_ref_write(): inline function
>  - rename_tmp_log(): improve error reporting
>  - rename_tmp_log(): use raceproof_create_file()
>  - lock_ref_sha1_basic(): use raceproof_create_file()
>  - lock_ref_sha1_basic(): inline constant
>  - raceproof_create_file(): new function
>  - safe_create_leading_directories(): set errno on SCLD_EXISTS
>  - safe_create_leading_directories_const(): preserve errno
>  - t5505: use "for-each-ref" to test for the non-existence of references
>  - refname_is_safe(): correct docstring
>  - files_rename_ref(): tidy up whitespace
> 
>  Deletion of a branch "foo/bar" could remove .git/refs/heads/foo
>  once there no longer is any other branch whose name begins with
>  "foo/", but we didn't do so so far.  Now we do.
> 
>  Expecting a reroll.
>  cf. <5051c78e-51f9-becd-e1a6-9c0b781d6...@alum.mit.edu>

I think you missed v4 of this patch series [1], which is the re-roll
that you were waiting for. And I missed that you missed it...

Michael

[1] http://public-inbox.org/git/cover.1483719289.git.mhag...@alum.mit.edu/

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Johannes Schindelin]

Hi Junio,

On Mon, 6 Feb 2017, Junio C Hamano wrote:

> * sf/putty-w-args (2017-02-01) 5 commits
>  - SQUASH???
>  - connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config
>  - git_connect(): factor out SSH variant handling
>  - connect: rename tortoiseplink and putty variables
>  - connect: handle putty/plink also in GIT_SSH_COMMAND
> 
>  The command line options for ssh invocation needs to be tweaked for
>  some implementations of SSH (e.g. PuTTY plink wants "-P "
>  while OpenSSH wants "-p " to specify port to connect to), and
>  the variant was guessed when GIT_SSH environment variable is used
>  to specify it.  Extend the guess to the command specified by the
>  newer GIT_SSH_COMMAND and also core.sshcommand configuration
>  variable, and give an escape hatch for users to deal with
>  misdetected cases.
> 
>  Stalled?
>  cf. 

The latest messages in that thread are

- your claim that you never said correctness is pused to a back seat (when
  an earlier, detailed mail listed four priorities of your patch review,
  none of which is said correctness, so I did not bother to answer), and

- my answer that suggested to take a break because the conversation turned
  less rational: I had to point out that your objection was not really
  valid in this case.

I now see that you added a SQUASH commit (that was news to me, thank you
very much), and that you seem to still insist that the code should prepare
for possible future changes in the config settings that may actually never
materialize. (And that would have to be handled at a different point, as I
had pointed out, so that suggested preparation would most likely not help
at all.)

In short: unless I read any convincing argument in favor of said SQUASH
commit, I will remain convinced that v3, as submitted, is actually the
best way forward.

Thank you for your attention,
Johannes

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Junio C Hamano]

Jeff King  writes:

> On Mon, Feb 06, 2017 at 02:34:08PM -0800, Junio C Hamano wrote:
>
>> * sk/parse-remote-cleanup (2017-02-06) 1 commit
>>   (merged to 'next' on 2017-02-06 at 6ec89f72d5)
>>  + parse-remote: remove reference to unused op_prep
>> 
>>  Code clean-up.
>> 
>>  Will merge to 'master'.
>
> Hrm. Are the functions in git-parse-remote.sh part of the public API?
> That is, do we expect third-party scripts to do:
>
>   . "$(git rev-parse --exec)/git-parse-remote.sh
>   error_on_missing_default_upstream "$a" "$b" "$c" "$d"
>
> ? If so, then they may be surprised by the change in function signature.
>
> I generally think of git-sh-setup as the one that external scripts would
> use. There _is_ a manpage for git-parse-remote, but it doesn't list any
> functions. So maybe they're all fair game for changing?
>
> I just didn't see any discussion of this in the original patch thread,
> so I wanted to make sure we were making that decision consciously, and
> not accidentally. :)

Ummm, yes, I admit that this was accidental.  I didn't really think
of parse-remote as an externally visible and supported interface,
but users have tendency to break our expectations, so, I dunno.

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Jeff King]

On Mon, Feb 06, 2017 at 02:34:08PM -0800, Junio C Hamano wrote:

> * sk/parse-remote-cleanup (2017-02-06) 1 commit
>   (merged to 'next' on 2017-02-06 at 6ec89f72d5)
>  + parse-remote: remove reference to unused op_prep
> 
>  Code clean-up.
> 
>  Will merge to 'master'.

Hrm. Are the functions in git-parse-remote.sh part of the public API?
That is, do we expect third-party scripts to do:

  . "$(git rev-parse --exec)/git-parse-remote.sh
  error_on_missing_default_upstream "$a" "$b" "$c" "$d"

? If so, then they may be surprised by the change in function signature.

I generally think of git-sh-setup as the one that external scripts would
use. There _is_ a manpage for git-parse-remote, but it doesn't list any
functions. So maybe they're all fair game for changing?

I just didn't see any discussion of this in the original patch thread,
so I wanted to make sure we were making that decision consciously, and
not accidentally. :)

-Peff

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Junio C Hamano]

Stefan Beller  writes:

>> * sb/submodule-doc (2017-01-12) 3 commits
>>  - submodules: add a background story
>>  - submodule update documentation: don't repeat ourselves
>>  - submodule documentation: add options to the subcommand
>
> The first two commits are good to go IIRC as you seemed to be
> positive about them at the time. Though I have a hard time finding
> evidence of such.
>
> I am currently reworking the 3/3 "add a background story" patch as it is
> RFC-ish, so no need to review that any more.
>
> Maybe we can get 1&2 merged and then 3 comes on its own?

Yeah, sorry, I keep forgetting this topic, it seems.  Let's advance
the earlier two.  Tentatively I'd drop the third one as you plan to
redo it separately.

Thanks for prodding.

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Stefan Beller]

> * sb/submodule-doc (2017-01-12) 3 commits
>  - submodules: add a background story
>  - submodule update documentation: don't repeat ourselves
>  - submodule documentation: add options to the subcommand
>
>  Needs review.
>

The first two commits are good to go IIRC as you seemed to be
positive about them at the time. Though I have a hard time finding
evidence of such.

I am currently reworking the 3/3 "add a background story" patch as it is
RFC-ish, so no need to review that any more.

Maybe we can get 1&2 merged and then 3 comes on its own?

Thanks,
Stefan

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Junio C Hamano]

SZEDER Gábor  writes:

> All failing tests fail with the same error:
>
>   fatal: unrecognized %(refname:strip=2) argument: strip=2
>
> That's because of this topic:
>
>> * kn/ref-filter-branch-list (2017-01-31) 20 commits

Ahh, of course.

Let's make sure the series won't escape to 'master' before the
"strip" breakage is fixed.  How about queuing this on top of the
ref-filter topic?  

It seems to unblock your completion-refs-speedup topic and makes the
test pass ;-)

Thanks.

-- >8 --
Subject: [PATCH] ref-filter: resurrect "strip" as a synonym to "lstrip"

We forgot that "strip" was introduced at 0571979bd6 ("tag: do not
show ambiguous tag names as "tags/foo"", 2016-01-25) as part of Git
2.8 (and 2.7.1), yet in the update to ref-filter, we started calling
it "lstrip" to make it easier to explain the new "rstrip" operation.

We shouldn't have renamed the existing one; "lstrip" should have
been a new synonym that means the same thing as "strip".  Scripts
in the wild are surely using the original form already.

Signed-off-by: Junio C Hamano 
---
 Documentation/git-for-each-ref.txt |  2 ++
 ref-filter.c   |  3 ++-
 t/t6300-for-each-ref.sh| 12 
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 2008600e7e..111e1be6f5 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -107,6 +107,8 @@ refname::
enough components, the result becomes an empty string if
stripping with positive , or it becomes the full refname if
stripping with negative .  Neither is an error.
++
+`strip` can be used as a synomym to `lstrip`.
 
 objecttype::
The type of the object (`blob`, `tree`, `commit`, `tag`).
diff --git a/ref-filter.c b/ref-filter.c
index 01b5c18ef0..2a94d6da98 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -112,7 +112,8 @@ static void refname_atom_parser_internal(struct 
refname_atom *atom,
atom->option = R_NORMAL;
else if (!strcmp(arg, "short"))
atom->option = R_SHORT;
-   else if (skip_prefix(arg, "lstrip=", )) {
+   else if (skip_prefix(arg, "lstrip=", ) ||
+skip_prefix(arg, "strip=", )) {
atom->option = R_LSTRIP;
if (strtol_i(arg, 10, >lstrip))
die(_("Integer value expected refname:lstrip=%s"), arg);
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 25a9973ce9..c87dc1f8bc 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -59,18 +59,26 @@ test_atom head refname:rstrip=1 refs/heads
 test_atom head refname:rstrip=2 refs
 test_atom head refname:rstrip=-1 refs
 test_atom head refname:rstrip=-2 refs/heads
+test_atom head refname:strip=1 heads/master
+test_atom head refname:strip=2 master
+test_atom head refname:strip=-1 master
+test_atom head refname:strip=-2 heads/master
 test_atom head upstream refs/remotes/origin/master
 test_atom head upstream:short origin/master
 test_atom head upstream:lstrip=2 origin/master
 test_atom head upstream:lstrip=-2 origin/master
 test_atom head upstream:rstrip=2 refs/remotes
 test_atom head upstream:rstrip=-2 refs/remotes
+test_atom head upstream:strip=2 origin/master
+test_atom head upstream:strip=-2 origin/master
 test_atom head push refs/remotes/myfork/master
 test_atom head push:short myfork/master
 test_atom head push:lstrip=1 remotes/myfork/master
 test_atom head push:lstrip=-1 master
 test_atom head push:rstrip=1 refs/remotes/myfork
 test_atom head push:rstrip=-1 refs
+test_atom head push:strip=1 remotes/myfork/master
+test_atom head push:strip=-1 master
 test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectname $(git rev-parse refs/heads/master)
@@ -636,6 +644,10 @@ EOF
 test_expect_success 'Verify usage of %(symref:lstrip) atom' '
git for-each-ref --format="%(symref:lstrip=2)" refs/heads/sym > actual 
&&
git for-each-ref --format="%(symref:lstrip=-2)" refs/heads/sym >> 
actual &&
+   test_cmp expected actual &&
+
+   git for-each-ref --format="%(symref:strip=2)" refs/heads/sym > actual &&
+   git for-each-ref --format="%(symref:strip=-2)" refs/heads/sym >> actual 
&&
test_cmp expected actual
 '
 
-- 
2.12.0-rc0-144-g99fe1a5456

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Jacob Keller]

On Mon, Feb 6, 2017 at 4:24 PM, SZEDER Gábor  wrote:
>> * sg/completion-refs-speedup (2017-02-06) 13 commits
>>  - squash! completion: fill COMPREPLY directly when completing refs
>>  - completion: fill COMPREPLY directly when completing refs
>>  - completion: list only matching symbolic and pseudorefs when completing 
>> refs
>>  - completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
>>  - completion: let 'for-each-ref' filter remote branches for 'checkout' 
>> DWIMery
>>  - completion: let 'for-each-ref' strip the remote name from remote branches
>>  - completion: let 'for-each-ref' and 'ls-remote' filter matching refs
>>  - completion: don't disambiguate short refs
>>  - completion: don't disambiguate tags and branches
>>  - completion: support excluding full refs
>>  - completion: support completing full refs after '--option=refs/'
>>  - completion: wrap __git_refs() for better option parsing
>>  - completion: remove redundant __gitcomp_nl() options from _git_commit()
>>  (this branch uses sg/completion.)
>>
>>  Will hold.
>>  This seems to break 9902 when merged to 'pu'.
>
> All failing tests fail with the same error:
>
>   fatal: unrecognized %(refname:strip=2) argument: strip=2
>
> That's because of this topic:
>
>> * kn/ref-filter-branch-list (2017-01-31) 20 commits
>>   (merged to 'next' on 2017-01-31 at e7592a5461)
>>  + branch: implement '--format' option
>>  + branch: use ref-filter printing APIs
>>  + branch, tag: use porcelain output
>>  + ref-filter: allow porcelain to translate messages in the output
>>  + ref-filter: add an 'rstrip=' option to atoms which deal with refnames
>>  + ref-filter: modify the 'lstrip=' option to work with negative ''
>>  + ref-filter: Do not abruptly die when using the 'lstrip=' option
>>  + ref-filter: rename the 'strip' option to 'lstrip'
>
> And in particular this commit, which, well, does what it's subject
> says it's doing, thus breaking backwards compatibility.
>

What about making strip a deprecated variant of lstrip?

Thanks,
Jake

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[SZEDER Gábor]

> * sg/completion-refs-speedup (2017-02-06) 13 commits
>  - squash! completion: fill COMPREPLY directly when completing refs
>  - completion: fill COMPREPLY directly when completing refs
>  - completion: list only matching symbolic and pseudorefs when completing refs
>  - completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
>  - completion: let 'for-each-ref' filter remote branches for 'checkout' 
> DWIMery
>  - completion: let 'for-each-ref' strip the remote name from remote branches
>  - completion: let 'for-each-ref' and 'ls-remote' filter matching refs
>  - completion: don't disambiguate short refs
>  - completion: don't disambiguate tags and branches
>  - completion: support excluding full refs
>  - completion: support completing full refs after '--option=refs/'
>  - completion: wrap __git_refs() for better option parsing
>  - completion: remove redundant __gitcomp_nl() options from _git_commit()
>  (this branch uses sg/completion.)
>
>  Will hold.
>  This seems to break 9902 when merged to 'pu'.

All failing tests fail with the same error:

  fatal: unrecognized %(refname:strip=2) argument: strip=2

That's because of this topic:

> * kn/ref-filter-branch-list (2017-01-31) 20 commits
>   (merged to 'next' on 2017-01-31 at e7592a5461)
>  + branch: implement '--format' option
>  + branch: use ref-filter printing APIs
>  + branch, tag: use porcelain output
>  + ref-filter: allow porcelain to translate messages in the output
>  + ref-filter: add an 'rstrip=' option to atoms which deal with refnames
>  + ref-filter: modify the 'lstrip=' option to work with negative ''
>  + ref-filter: Do not abruptly die when using the 'lstrip=' option
>  + ref-filter: rename the 'strip' option to 'lstrip'

And in particular this commit, which, well, does what it's subject
says it's doing, thus breaking backwards compatibility.


>  + ref-filter: make remote_ref_atom_parser() use 
> refname_atom_parser_internal()
>  + ref-filter: introduce refname_atom_parser()
>  + ref-filter: introduce refname_atom_parser_internal()
>  + ref-filter: make "%(symref)" atom work with the ':short' modifier
>  + ref-filter: add support for %(upstream:track,nobracket)
>  + ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
>  + ref-filter: introduce format_ref_array_item()
>  + ref-filter: move get_head_description() from branch.c
>  + ref-filter: modify "%(objectname:short)" to take length
>  + ref-filter: implement %(if:equals=) and %(if:notequals=)
>  + ref-filter: include reference to 'used_atom' within 'atom_value'
>  + ref-filter: implement %(if), %(then), and %(else) atoms
>
>  The code to list branches in "git branch" has been consolidated
>  with the more generic ref-filter API.
>
>  Will cook in 'next'.