Does it make sense to show tips on blame?

[Edmundo Carmona Antoranz]

Hi!

I'm having fun with difflame. I added support for an option called
'tips' which would display 1-line summary of a block of added lines
that belong to the same revision, in order to provide context while
reading difflame output without having to run an additional git show
command.

Output is like this (from README.txt, taken from difflame on git
itself, sorry if it's too wide):

diff --git a/fast-import.c b/fast-import.c
index f561ba833..64fe602f0 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2218,13 +2218,17 @@ static uintmax_t do_change_note_fanout(
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2218)
char *fullpath, unsigned int fullpath_len,
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2219)
unsigned char fanout)
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2220) {
-02d0457eb4 (Junio C Hamano 2017-01-10 15:24:26 -0800 2221) struct
tree_content *t = root->tree;
405d7f4af fast-import: properly fanout notes when tree is imported
+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2221)  struct
tree_content *t;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 )  struct
tree_entry *e, leaf;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2223)
unsigned int i, tmp_hex_sha1_len, tmp_fullpath_len;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2224)
uintmax_t num_notes = 0;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2225)
unsigned char sha1[20];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2226)  char
realpath[60];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2227)
405d7f4af fast-import: properly fanout notes when tree is imported
+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2228)  if (!root->tree)
+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2229)
 load_tree(root);
+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2230)  t = root->tree;
+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2231)
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2232)  for (i
= 0; t && i < t->entry_count; i++) {
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2233)
e = t->entries[i];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2234)
tmp_hex_sha1_len = hex_sha1_len + e->name->str_len;

The tip that was used for revision 405d7f4af on both "blocks" of added
lines can be seen.

The question is if you think it makes sense to add this option to
git-blame itself.

Something like:
git blame --tips README.txt

Thanks in advance

[PATCH v2 4/4] show-ref: Inline show_one

[Vladimir Panteleev]

As the small function is now only called from a single place, there is
no point in keeping it as a separate function any longer.

Signed-off-by: Vladimir Panteleev 
---
 builtin/show-ref.c | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 3cf344d47..ec44164d8 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -17,15 +17,6 @@ static int deref_tags, show_head, tags_only, heads_only, 
found_match, verify,
 static const char **pattern;
 static const char *exclude_existing_arg;
 
-static void show_one(const char *refname, const struct object_id *oid)
-{
-   const char *hex = find_unique_abbrev(oid->hash, abbrev);
-   if (hash_only)
-   printf("%s\n", hex);
-   else
-   printf("%s %s\n", hex, refname);
-}
-
 static int show_ref(const char *refname, const struct object_id *oid,
int flag, void *cbdata)
 {
@@ -74,7 +65,11 @@ static int show_ref(const char *refname, const struct 
object_id *oid,
if (quiet)
return 0;
 
-   show_one(refname, oid);
+   hex = find_unique_abbrev(oid->hash, abbrev);
+   if (hash_only)
+   printf("%s\n", hex);
+   else
+   printf("%s %s\n", hex, refname);
 
if (!deref_tags)
return 0;
-- 
2.11.0

[PATCH v2 1/4] show-ref: Allow --head to work with --verify

[Vladimir Panteleev]

Previously, when --verify was specified, show-ref would use a separate
code path which ignored --head. Thus, "git show-ref HEAD" used with
"--verify" (because the user is not interested in seeing
refs/remotes/origin/HEAD), and used with "--head" (because the user
does not want HEAD to be filtered out), i.e. "git show-ref --head
--verify HEAD", did not work as expected.

Instead of insisting that the input begins with "refs/", allow "HEAD"
when "--head" is given in the codepath that handles "--verify", so
that all valid full refnames including HEAD are passed to the same
output machinery.

Signed-off-by: Vladimir Panteleev 
---
 builtin/show-ref.c  |  3 ++-
 t/t1403-show-ref.sh | 14 ++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 6d4e66900..945a483e3 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -202,7 +202,8 @@ int cmd_show_ref(int argc, const char **argv, const char 
*prefix)
while (*pattern) {
struct object_id oid;
 
-   if (starts_with(*pattern, "refs/") &&
+   if ((starts_with(*pattern, "refs/") ||
+(show_head && !strcmp(*pattern, "HEAD"))) &&
!read_ref(*pattern, oid.hash)) {
if (!quiet)
show_one(*pattern, );
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 7e10bcfe3..2fb5dc879 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -164,4 +164,18 @@ test_expect_success 'show-ref --heads, --tags, --head, 
pattern' '
test_cmp expect actual
 '
 
+test_expect_success 'show-ref --verify --head' '
+   echo $(git rev-parse HEAD) HEAD >expect &&
+   git show-ref --verify --head HEAD >actual &&
+   test_cmp expect actual &&
+
+   >expect &&
+
+   git show-ref --verify --head -q HEAD >actual &&
+   test_cmp expect actual &&
+
+   test_must_fail git show-ref --verify --head -q A >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.11.0

[PATCH v2 3/4] show-ref: Optimize show_ref a bit

[Vladimir Panteleev]

The inner `if (verify)' check was not being used before the preceding
commit, as show_ref was never being called from a code path where
verify was non-zero.

Adding a `!verify' check to the outer if skips an unnecessary string
comparison when verify is non-zero, and show_ref is already called
with a reference exactly matching pattern.

Signed-off-by: Vladimir Panteleev 
---
 builtin/show-ref.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index bcdc1a95e..3cf344d47 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -43,7 +43,7 @@ static int show_ref(const char *refname, const struct 
object_id *oid,
if (!match)
return 0;
}
-   if (pattern) {
+   if (pattern && !verify) {
int reflen = strlen(refname);
const char **p = pattern, *m;
while ((m = *p++) != NULL) {
@@ -54,9 +54,6 @@ static int show_ref(const char *refname, const struct 
object_id *oid,
continue;
if (len == reflen)
goto match;
-   /* "--verify" requires an exact match */
-   if (verify)
-   continue;
if (refname[reflen - len - 1] == '/')
goto match;
}
-- 
2.11.0

[PATCH v2 2/4] show-ref: Allow -d to work with --verify

[Vladimir Panteleev]

Use the same output machinery when --verify is absent or present,
which allows tag dereferencing (-d) to work with --verify. This is
useful when the user wishes to avoid the costly iteration of refs.

Signed-off-by: Vladimir Panteleev 
---
 builtin/show-ref.c  | 3 +--
 t/t1403-show-ref.sh | 9 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 945a483e3..bcdc1a95e 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -205,8 +205,7 @@ int cmd_show_ref(int argc, const char **argv, const char 
*prefix)
if ((starts_with(*pattern, "refs/") ||
 (show_head && !strcmp(*pattern, "HEAD"))) &&
!read_ref(*pattern, oid.hash)) {
-   if (!quiet)
-   show_one(*pattern, );
+   show_ref(*pattern, , 0, NULL);
}
else if (!quiet)
die("'%s' - not a valid ref", *pattern);
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 2fb5dc879..5c540e67f 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -97,6 +97,9 @@ test_expect_success 'show-ref -d' '
git show-ref -d refs/tags/A refs/tags/C >actual &&
test_cmp expect actual &&
 
+   git show-ref --verify -d refs/tags/A refs/tags/C >actual &&
+   test_cmp expect actual &&
+
echo $(git rev-parse refs/heads/master) refs/heads/master >expect &&
git show-ref -d master >actual &&
test_cmp expect actual &&
@@ -116,6 +119,12 @@ test_expect_success 'show-ref -d' '
test_cmp expect actual &&
 
test_must_fail git show-ref -d --verify heads/master >actual &&
+   test_cmp expect actual &&
+
+   test_must_fail git show-ref --verify -d A C >actual &&
+   test_cmp expect actual &&
+
+   test_must_fail git show-ref --verify -d tags/A tags/C >actual &&
test_cmp expect actual
 
 '
-- 
2.11.0

[PATCH v2] show-ref: Allow -d, --head to work with --verify

[Vladimir Panteleev]

This is the second take for "[PATCH] show-ref: Allow --head to work
with --verify". Thanks to Junio for his extensive feedback.

Re: [PATCH] show-ref: Allow --head to work with --verify

[Vladimir Panteleev]

On 2017-01-20 23:29, Junio C Hamano wrote:

By the way, I have no strong preference between "read-ref, check
quiet and show-one" and "show-ref", so if you make --verify to
consistently call "show_ref()" for both refs/heads/master and HEAD,
that is also perfectly fine.


This sounds like a good idea, as it will also allow -d and some other 
options to work with --verify (whatever use case there might be for 
that). Will resubmit with that.



I just do not want to see the same feature/codepath to call two
different implementations that happens to do the same thing today.


Understood and agreed.

--
Best regards,
 Vladimir

Re: Idea: Add a filter option to 'git rebase'

[Thomas Braun]

Am 20.01.2017 um 23:28 schrieb Philip Oakley:
> A recent question on stackoverflow
> http://stackoverflow.com/questions/41753252/drop-commits-by-commit-message-in-git-rebase
> sought to remove automatically commits that could be identified by
> relevant words in the commit message.
> 
> I had thought that the ubiquitous `git filter-branch` should be able to
> do this sort of thing. I was wrong. (It was pointed out to me that...)
> The man page notes that removing a commit via filter-branch does not
> remove the changes from following commits and directs readers to using
> `git rebase(1)`.
> 
> However the rebase command does not have any filter option to allow the
> automatic population of its TODO list with the appropriate
> pick/edit/drop/etc. values.

Well you can use an arbitrary shell command as editor, so something like

$ GIT_SEQUENCE_EDITOR="sed -i -re 's/^pick /edit /'" git rebase -i master

will change pick to edit of all commits.

Maybe that can be mentioned in the man page of rebase?

Re: [PATCH v2 2/2] grep: use '/' delimiter for paths

[Brandon Williams]

On 01/20, Junio C Hamano wrote:
> Stefan Hajnoczi  writes:
> 
> > If the tree contains a sub-directory then git-grep(1) output contains a
> > colon character instead of a path separator:
> >
> >   $ git grep malloc v2.9.3:t
> >   v2.9.3:t:test-lib.sh: setup_malloc_check () {
> >   $ git show v2.9.3:t:test-lib.sh
> >   fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3'
> >
> > This patch attempts to use the correct delimiter:
> >
> >   $ git grep malloc v2.9.3:t
> >   v2.9.3:t/test-lib.sh: setup_malloc_check () {
> >   $ git show v2.9.3:t/test-lib.sh
> >   (success)
> >
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  builtin/grep.c  | 4 +++-
> >  t/t7810-grep.sh | 5 +
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/builtin/grep.c b/builtin/grep.c
> > index 90a4f3d..7a7aab9 100644
> > --- a/builtin/grep.c
> > +++ b/builtin/grep.c
> > @@ -494,7 +494,9 @@ static int grep_object(struct grep_opt *opt, const 
> > struct pathspec *pathspec,
> >  
> > /* Add a delimiter if there isn't one already */
> > if (name[len - 1] != '/' && name[len - 1] != ':') {
> > -   strbuf_addch(, ':');
> > +   /* rev: or rev:path/ */
> > +   char delim = obj->type == OBJ_COMMIT ? ':' : 
> > '/';
> 
> Why check the equality with commit, rather than un-equality with
> tree?  Wouldn't you want to treat $commit:path and $tag:path the
> same way?

I assume Stefan just grabbed my naive suggestion hence why it checks
equality with a commit.  So that's my fault :)  Either of these may
not be enough though, since if you do 'git grep malloc v2.9.3^{tree}'
with this change the output prefix is 'v2.9.3^{tree}/' instead of the
correct prefix 'v2.9.3^{tree}:'

> 
> I also think the two patches should be squashed together, as it is
> only after this patch that the code does the "right thing" by
> changing the delimiter depending on obj->type.
> 
> > +   strbuf_addch(, delim);
> > }
> > }
> > init_tree_desc(, data, size);
> > diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> > index e804a3f..8a58d5e 100755
> > --- a/t/t7810-grep.sh
> > +++ b/t/t7810-grep.sh
> > @@ -1445,6 +1445,11 @@ test_expect_success 'grep outputs valid : 
> > for HEAD:t/' '
> > test_cmp expected actual
> >  '
> >  
> > +test_expect_success 'grep outputs valid : for HEAD:t' '
> > +   git grep vvv HEAD:t >actual &&
> > +   test_cmp expected actual
> > +'
> > +
> 
> Perhaps you want an annotated tag object refs/tags/v1.0 that points
> at the commit at the HEAD, and then run the same grep on v1.0:t, in
> addition to the above test.
> 
> >  cat >expected < >  HEAD:t/a/v:vvv
> >  HEAD:t/v:vvv

-- 
Brandon Williams

Re: [PATCH] show-ref: Allow --head to work with --verify

[Junio C Hamano]

Junio C Hamano  writes:

> If two codepaths are called "I don't see a meaningful difference",
> then it is really better to share the same code.  Today, they may
> happen to behave identically.  When we need to update the behaviour
> of one, we'd be forced to update the other one to match.
>
> IOW, something along this line, perhaps (not even compile tested so
> take it with grain of salt).

By the way, I have no strong preference between "read-ref, check
quiet and show-one" and "show-ref", so if you make --verify to
consistently call "show_ref()" for both refs/heads/master and HEAD,
that is also perfectly fine.

I just do not want to see the same feature/codepath to call two
different implementations that happens to do the same thing today.

Thanks.

Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

[Stefan Beller]

On Fri, Jan 20, 2017 at 1:58 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> Jeff King  writes:
>>
 And in my current understanding of submodules the check in
 .gitmodules ought to be enough, too.
>>>
>>> Yeah, that probably makes sense. You can have a gitlink without a
>>> .gitmodules file, but I don't quite know what that would mean in terms
>>> of submodules (I guess it's not a submodule but "something else").
>>
>> That may be a lot better than reading the index unconditionally, but
>> I'd rather not to see "git rev-parse" read ".gitmodules" at all.  It
>> would discourage scripted use of Git for no good reason.
>
> Thinking about this more, I suspect that
>
> cd sub && git anything
>
> when the index of the top-level thinks "sub" must be a submodule and
> the user is not interested in "sub" (hence it hasn't gone through
> "git submodule init" or "update") should get the same error as you
> would get if you did
>
> cd /var/tmp/ && git anything
>
> when none of /, /var, /var/tmp/ is controlled by any Git repository.
> I.e. "fatal: Not a git repository".

I agree. The idea with a tombstone sounds great from a
performance perspective as you do not need to do extra work
in the superproject at all, because any gitlink is detected early
in the discovery.

The big BUT is however the following:
How do current users know if a submodule is e.g. populated?
(From say a third party script). Most likely they use something like

test -e ${sub}/.git

as that just worked. So if we go with the tombstone idea, we
may break things. (i.e. the fictional third party script confirms any
submodule to be there, but it is not)

I do really like the idea though, so maybe we also need to provide
some submodule plumbing that we opine to be the "correct" way
to see the submodules state[1] to make the transition easier for the
script writers?

[1] c.f. submodule states in
https://github.com/gitster/git/commit/e2b51b9df618ceeff7c4ec044e20f5ce9a87241e

>
> Perhaps we can update two things and make it cheap.
>
>  - checking out the top-level working tree without populating the
>working tree of a submodule learns to do a bit more than just
>creating an empty directory.  Instead, it creates the third kind
>of ".git" (we currently support two kinds of ".git", one that is
>a repository itself, and another that is points at a repository),
>that tells us that there is not (yet) a repository there.
>
>  - the "discovering the root of the working tree" logic learns to
>notice the third kind of ".git" and stop with "Not a git
>repository".

Re: [PATCH] show-ref: Allow --head to work with --verify

[Junio C Hamano]

Vladimir Panteleev  writes:

> --quiet will still work correctly with the current patch, because
> show_ref already checks quiet. Granted, the original --verify code
> used show_one and not show_ref; however, I don't see a meaningful
> difference between calling show_ref and show_one for HEAD, other than
> a bit of overhead, so adding a new function may not be worthwhile. I
> will still add tests for this; however, in light of this, would you
> still like me to perform the change you requested?

If two codepaths are called "I don't see a meaningful difference",
then it is really better to share the same code.  Today, they may
happen to behave identically.  When we need to update the behaviour
of one, we'd be forced to update the other one to match.

IOW, something along this line, perhaps (not even compile tested so
take it with grain of salt).

Thanks.

 builtin/show-ref.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 6d4e669002..57491152b7 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -202,7 +202,8 @@ int cmd_show_ref(int argc, const char **argv, const char 
*prefix)
while (*pattern) {
struct object_id oid;
 
-   if (starts_with(*pattern, "refs/") &&
+   if (((show_head && !strcmp(*pattern, "HEAD")) ||
+starts_with(*pattern, "refs/")) &&
!read_ref(*pattern, oid.hash)) {
if (!quiet)
show_one(*pattern, );

Re: [PATCH v2 0/2] grep: make output consistent with revision syntax

[Junio C Hamano]

Stefan Hajnoczi  writes:

> v2:
>  * Use obj->type instead of re-parsing name for delimiter
>(Followed Brandon's suggestion but named the variable 'delim' since that
>name is used in other places and 'del' is used for deletion.)
>  * Add tests
>  * Update Patch 1 commit description with a more relevant example
>  * PATCH instead of RFC, now works with all documented git-rev-parse(1) syntax
>
> git-grep(1)'s output is not consistent with git-rev-parse(1) revision syntax.

While I was queuing this series (which I think should become a
single patch in the final version), I was trying to see how it
should be described in the release note (aka an entry in the
periodicall "What's cooking" report).  Here is how I explained it.
You may want to borrow some parts of the description, especially the
part that talks about : being a way to name a blob,
when updating the commit log message.

 "git grep", when fed a tree-ish as an input, shows each hit
 prefixed with ":::".  As  is
 almost always either a commit or a tag that points at a commit,
 the early part of the output ":" can be used as
 the name of the blob and given to "git show".  

 When  is a tree given in the extended SHA-1 syntax
 (e.g. ":", or ":"), however, this results
 in a string that does not name a blob (e.g. "::"
 or "::").  "git grep" has been taught to be
 a bit more intelligent about these cases and omit a colon (in
 the former case) or use slash (in the latter case) to produce
 ":" and ":/" that can be used
 as the name of a blob.

As a release note entry is written in a style different from the
commit log message, you would need to adjust the voice of the last
sentence (i.e. "Teach 'git grep' to ..." to give an order to the
codebase), but otherwise the above would make an understandable
justification for the change suitable in a log message.

Re: [PATCH] show-ref: Allow --head to work with --verify

[Junio C Hamano]

Junio C Hamano  writes:

> Your log message for the patch needs to be updated by summarizing
> the above better.

Here is my attempt.

 "git show-ref HEAD" used with "--verify" (because the user is
 not interested in seeing refs/remotes/origin/HEAD), and used
 with "--head" (because the user does not want HEAD to be
 filtered out), i.e. "git show-ref --head --verify HEAD", did
 not work as expected.

 Instead of insisting that the input begins with "refs/", allow
 "HEAD" when "--head" is given in the codepath that handles
 "--verify", so that all valid full refnames including HEAD are
 passed to the same output machinery.

Re: [RFC 2/2] grep: use '/' delimiter for paths

[Junio C Hamano]

Jeff King  writes:

> It's not ignored; just as with git-log, it's a pathspec to limit the
> diff. E.g.:
>
>   $ git show --name-status v2.9.3
>   ...
>   M   Documentation/RelNotes/2.9.3.txt
>   M   Documentation/git.txt
>   M   GIT-VERSION-GEN
>
>   $ git show --name-status v2.9.3 -- Documentation
>   M   Documentation/RelNotes/2.9.3.txt
>   M   Documentation/git.txt
>
> That's typically less useful than it is with log (where limiting the
> diff also kicks in history simplification and omits some commits
> entirely). But it does do something.

I think Stefan is missing the fact that the argument to "git show
:" actually is naming a blob that sits at the 
in the .  In other words, "show" is acting as a glorified
"git -p cat-file blob", in that use.

The use of "git show" you are demonstrating is still about showing
the commit object, whose behaviour is defined to show the log
message and the diff relative to its sole parent, limited to the
paths that match the pathspec.

It is perfectly fine and desirable that "git show :"
and "git show  -- " behaves differently.  These are
two completely different features.

Re: [PATCH] show-ref: Allow --head to work with --verify

[Vladimir Panteleev]

On 2017-01-20 19:03, Junio C Hamano wrote:

and viewed in the wider context, I notice that quiet is not honored
in the added code.  I think that is easily fixable by replacing this
hunk with something like:


--quiet will still work correctly with the current patch, because 
show_ref already checks quiet. Granted, the original --verify code used 
show_one and not show_ref; however, I don't see a meaningful difference 
between calling show_ref and show_one for HEAD, other than a bit of 
overhead, so adding a new function may not be worthwhile. I will still 
add tests for this; however, in light of this, would you still like me 
to perform the change you requested?


--
Best regards,
 Vladimir

Re: [PATCH] show-ref: Allow --head to work with --verify

[Junio C Hamano]

Junio C Hamano  writes:

>> My use case was the following series of steps:
>> ... long and readable if a bit too verbose description ...
> Your log message for the patch needs to be updated by summarizing
> the above better.

That raises the number of things to improve in the patch to 3 (so
far):

 * the log message clarification.

 * the code change I mentioned already to hook into the existing
   "only read the fully specified ref" part, sharing the actual
   action (i.e. read_ref() to read it, honor quiet and show it by
   calling show_one()), instead of adding another clause that does
   it differently (i.e. you didn't do read_ref/quiet?/show_one)

 * add a test to make sure that the command also works sensibly when
   --quiet is used.

With all of the above, I think the change makes sense.

Idea: Add a filter option to 'git rebase'

[Philip Oakley]

A recent question on stackoverflow 
http://stackoverflow.com/questions/41753252/drop-commits-by-commit-message-in-git-rebase 
sought to remove automatically commits that could be identified by relevant 
words in the commit message.


I had thought that the ubiquitous `git filter-branch` should be able to do 
this sort of thing. I was wrong. (It was pointed out to me that...) The man 
page notes that removing a commit via filter-branch does not remove the 
changes from following commits and directs readers to using `git rebase(1)`.


However the rebase command does not have any filter option to allow the 
automatic population of its TODO list with the appropriate 
pick/edit/drop/etc. values.


It does feel as if a --filter style option would be a useful addition to 
rebase to complement the filter-branch options once the current conversion 
from script to code is complete.


Is this something that should be put in the 'worth considering' pile?

--
Philip 

Re: [PATCH] show-ref: Allow --head to work with --verify

[Junio C Hamano]

Vladimir Panteleev  writes:

> Hi Junio,
>
> On 2017-01-20 19:03, Junio C Hamano wrote:
>> Having said all that, using --verify on HEAD does not make much
>> sense, because if HEAD is missing in .git/, I do not think Git
>> considers that directory as a Git repository to begin with.  So from
>> that point of view, I am not sure what value this change adds to the
>> system, even though the change is almost correct (modulo the "quiet"
>> thing).
>
> My use case was the following series of steps:
>
> Q1: How do I resolve a full ref name to a commit SHA1?
> A1: Use show-ref .
>
> Q2: How to make git show-ref also work when HEAD is specified as the
> reference?
> A2: Add --head.
>
> Q3: How do I make git show-ref only look for the exact full ref name
> specified, instead of doing a pattern/substring search, and thus
> output at most one result?
> A3: Add --verify.
>
> However, A2 and A3 are incompatible. Thus, there doesn't seem to be a
> way to e.g. make a simple alias which looks up a ref only by its full
> ref name, where said ref might or might not be HEAD. The obvious
> workaround is to check if the ref is HEAD in the rev-parse caller,
> however it seemed more logical to fix it in git instead.
>
> The documentation for show-ref also makes no mention that --head is
> ignored if --verify is specified, and the combination was not covered
> by any tests, therefore this seemed to me as a simple omission in
> --verify's logic.
>
> There is also rev-parse, which also has a --verify switch, however it
> does something else, and I don't see a way to ask rev-parse to only
> consider full refs.

Your log message for the patch needs to be updated by summarizing
the above better.  I couldn't read the motivation behind the change
fully from the original (even though I guessed it correctly).

Re: [PATCH v2 1/2] grep: only add delimiter if there isn't one already

[Junio C Hamano]

Stefan Hajnoczi  writes:

> git-grep(1) output does not follow git's own syntax:
>
>   $ git grep malloc v2.9.3:t/
>   v2.9.3:t/:test-lib.sh:  setup_malloc_check () {
>   $ git show v2.9.3:t/:test-lib.sh
>   fatal: Path 't/:test-lib.sh' does not exist in 'v2.9.3'
>
> This patch avoids emitting the unnecessary ':' delimiter if the name
> already ends with ':' or '/':
>
>   $ git grep malloc v2.9.3:
>   v2.9.3:t/test-lib.sh:   setup_malloc_check () {
>   $ git show v2.9.3:t/test-lib.sh
>   (succeeds)

>
> Signed-off-by: Stefan Hajnoczi 
> ---
>  builtin/grep.c  |  6 +-
>  t/t7810-grep.sh | 21 +
>  2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 8887b6a..90a4f3d 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -491,7 +491,11 @@ static int grep_object(struct grep_opt *opt, const 
> struct pathspec *pathspec,
>   strbuf_init(, PATH_MAX + len + 1);
>   if (len) {
>   strbuf_add(, name, len);
> - strbuf_addch(, ':');
> +
> + /* Add a delimiter if there isn't one already */
> + if (name[len - 1] != '/' && name[len - 1] != ':') {
> + strbuf_addch(, ':');
> + }

I agree with peff and Brandon that checking treeness is the right
way to make the decision, and you seem to have done that in the
updated 2/2, but why doesn't it apply here as well?


>   }
>   init_tree_desc(, data, size);
>   hit = grep_tree(opt, pathspec, , , base.len,

Re: [PATCH v2 2/2] grep: use '/' delimiter for paths

[Junio C Hamano]

Stefan Hajnoczi  writes:

> If the tree contains a sub-directory then git-grep(1) output contains a
> colon character instead of a path separator:
>
>   $ git grep malloc v2.9.3:t
>   v2.9.3:t:test-lib.sh:   setup_malloc_check () {
>   $ git show v2.9.3:t:test-lib.sh
>   fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3'
>
> This patch attempts to use the correct delimiter:
>
>   $ git grep malloc v2.9.3:t
>   v2.9.3:t/test-lib.sh:   setup_malloc_check () {
>   $ git show v2.9.3:t/test-lib.sh
>   (success)
>
> Signed-off-by: Stefan Hajnoczi 
> ---
>  builtin/grep.c  | 4 +++-
>  t/t7810-grep.sh | 5 +
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 90a4f3d..7a7aab9 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -494,7 +494,9 @@ static int grep_object(struct grep_opt *opt, const struct 
> pathspec *pathspec,
>  
>   /* Add a delimiter if there isn't one already */
>   if (name[len - 1] != '/' && name[len - 1] != ':') {
> - strbuf_addch(, ':');
> + /* rev: or rev:path/ */
> + char delim = obj->type == OBJ_COMMIT ? ':' : 
> '/';

Why check the equality with commit, rather than un-equality with
tree?  Wouldn't you want to treat $commit:path and $tag:path the
same way?

I also think the two patches should be squashed together, as it is
only after this patch that the code does the "right thing" by
changing the delimiter depending on obj->type.

> + strbuf_addch(, delim);
>   }
>   }
>   init_tree_desc(, data, size);
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index e804a3f..8a58d5e 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -1445,6 +1445,11 @@ test_expect_success 'grep outputs valid : 
> for HEAD:t/' '
>   test_cmp expected actual
>  '
>  
> +test_expect_success 'grep outputs valid : for HEAD:t' '
> + git grep vvv HEAD:t >actual &&
> + test_cmp expected actual
> +'
> +

Perhaps you want an annotated tag object refs/tags/v1.0 that points
at the commit at the HEAD, and then run the same grep on v1.0:t, in
addition to the above test.

>  cat >expected <  HEAD:t/a/v:vvv
>  HEAD:t/v:vvv

Re: [PATCH] log: new option decorate reflog of remote refs

[Jacob Keller]

On Fri, Jan 20, 2017 at 6:30 AM, Jeff King  wrote:
>> Imposing order between options could cause confusion, I think, if you
>> remove --decorate-reflog leaving --remotes on by accident, now you get
>> --remotes with a new meaning. We could go with something like
>> --decodate-reflog=remote, but that clashes with the number of reflog
>> entries and we may need a separator, like --decorate-reflog=remote,3.
>> Or we could add something to --decorate= in addition to
>> short|full|auto|no. Something like --decorate=full,reflog or
>> --decorate=full,reflog=remote,entries=3 if I want 3 reflog entries.
>
> I agree that making option-order important is potentially confusing. But
> it does already exist with --exclude. It's necessary to specify some
> sets of refs (e.g., all of A, except for those that match B, and then
> all of C, including those that match B).
>
> Having --decorate-reflog=remote would be similarly constrained. You
> couldn't do "decorate all remotes except for these ones". For that
> matter, I'm not sure how you would do "decorate just the refs from
> origin".
>
> I'll grant that those are going to be a lot less common than just "all
> the remotes" (or all the tags, or whatever). I'd just hate to see us
> revisiting this in a year to generalize it, and being stuck with
> historical baggage.
>
>> My hesitant to go that far is because I suspect decorating reflog
>> won't be helpful for non-remotes. But I'm willing to make more changes
>> if it opens door to master.
>
> Forgetting reflogs for a moment, I'd actually find it useful to just
> decorate tags and local branches, but not remotes. But right now there
> isn't any way to select which refs are worthy of decoration (reflog or
> not).
>
> That's why I'm thinking so much about a general ref-selection system. I
> agree the "--exclude=... --remotes" thing is complicated, but it's also
> the ref-selection system we _already_ have, which to me is a slight
> point in its favor.
>
> -Peff

I agree that the interaction between --exclude and --remotes/etc is
confusing, but I think it's reasonable enough because we already
support it, so it makes sense to extend it with this. I also think its
better to extend here than it is to hard-code it. We could provide a
single short-option that does the longer variant if it's that common.

Thanks,
Jake

Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

[Jeff King]

On Fri, Jan 20, 2017 at 01:58:02PM -0800, Junio C Hamano wrote:

> Junio C Hamano  writes:
> 
> > Jeff King  writes:
> >
> >>> And in my current understanding of submodules the check in
> >>> .gitmodules ought to be enough, too.
> >>
> >> Yeah, that probably makes sense. You can have a gitlink without a
> >> .gitmodules file, but I don't quite know what that would mean in terms
> >> of submodules (I guess it's not a submodule but "something else").
> >
> > That may be a lot better than reading the index unconditionally, but
> > I'd rather not to see "git rev-parse" read ".gitmodules" at all.  It
> > would discourage scripted use of Git for no good reason.
> 
> Thinking about this more, I suspect that
> 
>   cd sub && git anything
> 
> when the index of the top-level thinks "sub" must be a submodule and
> the user is not interested in "sub" (hence it hasn't gone through
> "git submodule init" or "update") should get the same error as you
> would get if you did
> 
>   cd /var/tmp/ && git anything
> 
> when none of /, /var, /var/tmp/ is controlled by any Git repository.
> I.e. "fatal: Not a git repository".

Not sure if our emails just crossed, but yes, I agree completely.

> Perhaps we can update two things and make it cheap.
> 
>  - checking out the top-level working tree without populating the
>working tree of a submodule learns to do a bit more than just
>creating an empty directory.  Instead, it creates the third kind
>of ".git" (we currently support two kinds of ".git", one that is
>a repository itself, and another that is points at a repository),
>that tells us that there is not (yet) a repository there.
> 
>  - the "discovering the root of the working tree" logic learns to
>notice the third kind of ".git" and stop with "Not a git
>repository".

Yeah, I thought about suggesting something like that earlier. It's
slightly more efficient than the "find the root and then complain" thing
Stefan and I were talking about, but I'd worry it comes with weird
corner cases. E.g., are there tools (or other parts of git) that care
about it literally being an empty directory? E.g., would parts of
"checkout" need to know that it's OK to blow it away?

Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

[Junio C Hamano]

Junio C Hamano  writes:

> Jeff King  writes:
>
>>> And in my current understanding of submodules the check in
>>> .gitmodules ought to be enough, too.
>>
>> Yeah, that probably makes sense. You can have a gitlink without a
>> .gitmodules file, but I don't quite know what that would mean in terms
>> of submodules (I guess it's not a submodule but "something else").
>
> That may be a lot better than reading the index unconditionally, but
> I'd rather not to see "git rev-parse" read ".gitmodules" at all.  It
> would discourage scripted use of Git for no good reason.

Thinking about this more, I suspect that

cd sub && git anything

when the index of the top-level thinks "sub" must be a submodule and
the user is not interested in "sub" (hence it hasn't gone through
"git submodule init" or "update") should get the same error as you
would get if you did

cd /var/tmp/ && git anything

when none of /, /var, /var/tmp/ is controlled by any Git repository.
I.e. "fatal: Not a git repository".

Perhaps we can update two things and make it cheap.

 - checking out the top-level working tree without populating the
   working tree of a submodule learns to do a bit more than just
   creating an empty directory.  Instead, it creates the third kind
   of ".git" (we currently support two kinds of ".git", one that is
   a repository itself, and another that is points at a repository),
   that tells us that there is not (yet) a repository there.

 - the "discovering the root of the working tree" logic learns to
   notice the third kind of ".git" and stop with "Not a git
   repository".

Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

[Jeff King]

On Fri, Jan 20, 2017 at 01:41:54PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> >> And in my current understanding of submodules the check in
> >> .gitmodules ought to be enough, too.
> >
> > Yeah, that probably makes sense. You can have a gitlink without a
> > .gitmodules file, but I don't quite know what that would mean in terms
> > of submodules (I guess it's not a submodule but "something else").
> 
> That may be a lot better than reading the index unconditionally, but
> I'd rather not to see "git rev-parse" read ".gitmodules" at all.  It
> would discourage scripted use of Git for no good reason.

Why is that? Just because it makes rev-parse seem more bloated?

I think Stefan's putting it into git.c is confusing the issue a bit.
This is fundamentally about figuring out which git repository we're in,
and that procedure is the right place to put the check.

IOW, when we call setup_git_repository() we are already walking up the
tree and looking at .git/HEAD, .git/config, etc to see if we are in a
valid git repository. It doesn't seem unreasonable to me to make this
part of that check. I.e.:

  - if we we walked up from the working tree (so we have a non-NULL
prefix); and

  - if there is a .gitmodules file; and

  - if the .gitmodules file shows that we were inside what _should_ have
been a submodule; then

  - complain and do not accept the outer repository as a valid repo.

That adds only an extra failed open() for people who do not use
submodules, and an extra config-file parse for people who do. And then
only when they are not in the top-level of the working tree (so scripts,
etc that cd_to_toplevel wouldn't pay per-invocation).

-Peff

Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

[Junio C Hamano]

Jeff King  writes:

>> And in my current understanding of submodules the check in
>> .gitmodules ought to be enough, too.
>
> Yeah, that probably makes sense. You can have a gitlink without a
> .gitmodules file, but I don't quite know what that would mean in terms
> of submodules (I guess it's not a submodule but "something else").

That may be a lot better than reading the index unconditionally, but
I'd rather not to see "git rev-parse" read ".gitmodules" at all.  It
would discourage scripted use of Git for no good reason.

Re: [PATCH] show-ref: Allow --head to work with --verify

[Vladimir Panteleev]

And to clarify:

On 2017-01-20 20:26, Vladimir Panteleev wrote:

On 2017-01-20 19:03, Junio C Hamano wrote:

Having said all that, using --verify on HEAD does not make much
sense, because if HEAD is missing in .git/, I do not think Git
considers that directory as a Git repository to begin with.


The behavior of --verify I am interested in is not to check that the ref 
exists, but to get its SHA1 while avoiding the pattern search. This 
avoids accidentally matching refs via substring (e.g. "git show-ref 
--head HEAD" will print HEAD, but also e.g. refs/remotes/origin/HEAD), 
and for performance reasons (looking up a ref by exact name can be MUCH 
faster than matching all refs by pattern search, e.g. in one of my 
projects where I use git as an object store, --verify makes a difference 
of 21 milliseconds vs. over 5 minutes).


--
Best regards,
 Vladimir

Re: [PATCH] show-ref: Allow --head to work with --verify

[Vladimir Panteleev]

Hi Junio,

On 2017-01-20 19:03, Junio C Hamano wrote:

Having said all that, using --verify on HEAD does not make much
sense, because if HEAD is missing in .git/, I do not think Git
considers that directory as a Git repository to begin with.  So from
that point of view, I am not sure what value this change adds to the
system, even though the change is almost correct (modulo the "quiet"
thing).


My use case was the following series of steps:

Q1: How do I resolve a full ref name to a commit SHA1?
A1: Use show-ref .

Q2: How to make git show-ref also work when HEAD is specified as the 
reference?

A2: Add --head.

Q3: How do I make git show-ref only look for the exact full ref name 
specified, instead of doing a pattern/substring search, and thus output 
at most one result?

A3: Add --verify.

However, A2 and A3 are incompatible. Thus, there doesn't seem to be a 
way to e.g. make a simple alias which looks up a ref only by its full 
ref name, where said ref might or might not be HEAD. The obvious 
workaround is to check if the ref is HEAD in the rev-parse caller, 
however it seemed more logical to fix it in git instead.


The documentation for show-ref also makes no mention that --head is 
ignored if --verify is specified, and the combination was not covered by 
any tests, therefore this seemed to me as a simple omission in 
--verify's logic.


There is also rev-parse, which also has a --verify switch, however it 
does something else, and I don't see a way to ask rev-parse to only 
consider full refs.


--
Best regards,
 Vladimir

Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

[Stefan Beller]

On Fri, Jan 20, 2017 at 12:00 PM, Jeff King  wrote:
> On Fri, Jan 20, 2017 at 11:53:01AM -0800, Stefan Beller wrote:
>
>> > One alternative would be to make the check cheaper. Could we reliably
>> > tell from the submodule.foo.* block in the config that path "foo" is a
>> > submodule? I think that would work after "submodule init" but not right
>> > after "git clone". So the index really is the source of truth there.
>>
>> Well we can check if there is a .gitmodules file that has a
>> submodule.*.path equal to the last part of $CWD, no need to look
>> at the git config.
>>
>> And that would also work right after git clone (in an
>> unpopulated/uninitialized submodule as I call it).
>>
>> And in my current understanding of submodules the check in
>> .gitmodules ought to be enough, too.
>
> Yeah, that probably makes sense. You can have a gitlink without a
> .gitmodules file, but I don't quite know what that would mean in terms
> of submodules (I guess it's not a submodule but "something else").

yeah, I agree it could be git series[1] at work, or as you said
"something else", and we have no idea what to do.

I think this could actually be implemented top-down, because the
check is cheap as we're beginning with lstat(.gitmodules), and no further
pursue checking this corner case in case the .gitmodules is not found.

I'll see if I can make a patch that passes the test suite.

[1] https://github.com/git-series/git-series/blob/master/INTERNALS.md

>
> -Peff

Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

[Jeff King]

On Fri, Jan 20, 2017 at 11:53:01AM -0800, Stefan Beller wrote:

> > One alternative would be to make the check cheaper. Could we reliably
> > tell from the submodule.foo.* block in the config that path "foo" is a
> > submodule? I think that would work after "submodule init" but not right
> > after "git clone". So the index really is the source of truth there.
> 
> Well we can check if there is a .gitmodules file that has a
> submodule.*.path equal to the last part of $CWD, no need to look
> at the git config.
> 
> And that would also work right after git clone (in an
> unpopulated/uninitialized submodule as I call it).
> 
> And in my current understanding of submodules the check in
> .gitmodules ought to be enough, too.

Yeah, that probably makes sense. You can have a gitlink without a
.gitmodules file, but I don't quite know what that would mean in terms
of submodules (I guess it's not a submodule but "something else").

-Peff

Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

[Stefan Beller]

On Fri, Jan 20, 2017 at 11:42 AM, Jeff King  wrote:
> On Fri, Jan 20, 2017 at 11:33:45AM -0800, Stefan Beller wrote:
>
>> > I'd rather see it in the commands themselves. Especially given the
>> > "ideal" in your status example, which requires command-specific
>> > knowledge.
>>
>> So you rather want to go bottom up, i.e. add it to each command individually
>> for which it makes sense, instead of rather first having a catch-it-all like
>> this and then we can have a flag similar to RUN_SETUP, e.g.
>> ALLOW_IN_UNPOP_SUBMODULE, which allows commands to
>> take over the responsibility to act responsibly in this case?
>
> Yes. I know it's "less safe" in the sense that commands have to make an
> effort to detect the situation, but I feel like only they'll know what
> the sensible behavior is. And they can also do the check at a time when
> they would be reading the index anyway.
>
>> status may be the first command for going that route; I wonder if we'd
>> want to add this feature unconditionally or only in the porcelain case.
>> (In plumbing you're supposed to know what you're doing... so there is
>> no need as well as our promise to not change it)
>
> Yeah. The reason that it would be so painful to load the index
> for every rev-parse is not just that it probably doesn't otherwise need
> the index, but that scripts may make a _ton_ of rev-parse (or other
> plumbing) calls.
>
> One alternative would be to make the check cheaper. Could we reliably
> tell from the submodule.foo.* block in the config that path "foo" is a
> submodule? I think that would work after "submodule init" but not right
> after "git clone". So the index really is the source of truth there.

Well we can check if there is a .gitmodules file that has a
submodule.*.path equal to the last part of $CWD, no need to look
at the git config.

And that would also work right after git clone (in an
unpopulated/uninitialized submodule as I call it).

And in my current understanding of submodules the check in
.gitmodules ought to be enough, too.

>
> I guess there could be an index extension "these are the gitlinks I
> contain" and in theory we could read just that extension. I dunno.
>
> -Peff

Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

[Jeff King]

On Fri, Jan 20, 2017 at 11:33:45AM -0800, Stefan Beller wrote:

> > I'd rather see it in the commands themselves. Especially given the
> > "ideal" in your status example, which requires command-specific
> > knowledge.
> 
> So you rather want to go bottom up, i.e. add it to each command individually
> for which it makes sense, instead of rather first having a catch-it-all like
> this and then we can have a flag similar to RUN_SETUP, e.g.
> ALLOW_IN_UNPOP_SUBMODULE, which allows commands to
> take over the responsibility to act responsibly in this case?

Yes. I know it's "less safe" in the sense that commands have to make an
effort to detect the situation, but I feel like only they'll know what
the sensible behavior is. And they can also do the check at a time when
they would be reading the index anyway.

> status may be the first command for going that route; I wonder if we'd
> want to add this feature unconditionally or only in the porcelain case.
> (In plumbing you're supposed to know what you're doing... so there is
> no need as well as our promise to not change it)

Yeah. The reason that it would be so painful to load the index
for every rev-parse is not just that it probably doesn't otherwise need
the index, but that scripts may make a _ton_ of rev-parse (or other
plumbing) calls.

One alternative would be to make the check cheaper. Could we reliably
tell from the submodule.foo.* block in the config that path "foo" is a
submodule? I think that would work after "submodule init" but not right
after "git clone". So the index really is the source of truth there.

I guess there could be an index extension "these are the gitlinks I
contain" and in theory we could read just that extension. I dunno.

-Peff

Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

[Stefan Beller]

On Fri, Jan 20, 2017 at 11:17 AM, Jeff King  wrote:
> On Thu, Jan 19, 2017 at 11:30:23AM -0800, Stefan Beller wrote:
>
>> Now let's ask the same question for "git -C sub status ." (which is a
>> command that is only reading and not writing to the repository)
>>
>> 1) If the submodule is populated, the user clearly intended to know
>>more about the submodules status
>> 2) It is unclear if the user wanted to learn about the submodules state
>>(So ideally: "The submodule 'sub' is not initialized. To init ...")
>>or the status check should be applied to the superproject instead.
>>
>> Avoid the confusion in 2) as well and just error out for now. Later on
>> we may want to add another flag to git.c to allow commands to be run
>> inside unpopulated submodules and each command reacts appropriately.
>
> I like the general idea of catching commands in unpopulated submodules,
> but I'm somewhat uncomfortable with putting an unconditional check into
> git.c, for two reasons:
>
>   1. Reading the index can be expensive. You would not want "git
>  rev-parse" to incur this cost.

Well, I would want rev-parse to not be run in the wrong repo.
(intended to rev-parse something in the submodule, but got results for
the superproject).

Talking about rev-parse, I was about to propose an extension in reply to
"[PATCH] git-prompt.sh: add submodule indicator" that rev-parse could
learn a flag similar to --show-toplevel, named:
--show-superproject-if-any or
--indicate-if-in-submodule-possibly
which would help out there.

>
>   2. How does this interact with commands which do interact with the
>  index? Don't they expect to find the_index unpopulated?

That is another sloppiness in this RFC patch, as I haven't nailed down
the corner cases yet.

>
>  (I notice that it's effectively tied to RUN_SETUP, which is good.
>   But that also means that many commands, like "diff", won't get the
>   benefit. Not to mention non-builtins).
>
> I'd rather see it in the commands themselves. Especially given the
> "ideal" in your status example, which requires command-specific
> knowledge.

So you rather want to go bottom up, i.e. add it to each command individually
for which it makes sense, instead of rather first having a catch-it-all like
this and then we can have a flag similar to RUN_SETUP, e.g.
ALLOW_IN_UNPOP_SUBMODULE, which allows commands to
take over the responsibility to act responsibly in this case?

status may be the first command for going that route; I wonder if we'd
want to add this feature unconditionally or only in the porcelain case.
(In plumbing you're supposed to know what you're doing... so there is
no need as well as our promise to not change it)

Thanks,
Stefan

>
> -Peff

Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard

[Jeff King]

On Fri, Jan 20, 2017 at 11:16:15AM -0800, Jacob Keller wrote:

> > I recently taught urxvt to recognize sha1s and grab them via keyboard
> > hints, and I'm finding it quite useful. Here's what it looks like if
> > you're interested:
> >
> >   http://peff.net/git-hints.gif
> >
> > The hints technique is taken from pentadactyl (which I also use), but
> > the urxvt port is mine. I'm happy to share the code.
> [...]
> 
> I would be interested in the code for this.. I'm curious if I can
> adapt it to my use of tmux.

See below. You can drop it into ~/.urxvt/ext/urlmatch and put something
like this into your Xresources:

  URxvt.keysym.C-b: urlmatch:list
  URxvt.keysym.C-semicolon: urlmatch:hint

I think the interesting bits if you're adapting it to another system
will be the regexes in find_commits(), and the key-at-a-time state
machine in handle_select_key().

Note that there's a sort-of bug in find_commits(). If you have:

  1234abcd..5678abcd

it finds three matches: one for each sha1, and one for the whole range.
But the range hint is covered up by the first sha1's hint.

And probably other bugs, too. I've only been using it for about a week,
and I did a bunch of cleanups this morning (after foolishly offering to
let other people see it ;) ).

-- >8 --
#!/usr/bin/perl

use warnings qw(all FATAL);
use strict;

sub on_action {
my ($self, $action) = @_;
($action, my @args) = split /:/, $action;
if ($action eq 'list') {
$self->matchlist(@args);
} elsif ($action eq 'hint') {
$self->open_hints(@args);
}
();
}

sub d { print STDERR "debug: " . join(' ', @_) . "\n"; }

sub handle_select_key {
my ($self, $event, $keysym, $octets) = @_;

my $input = $self->{input};

my $escape;
if (48 <= $keysym && $keysym <= 57) {
$input->{number} .= ($keysym - 48);
} elsif (97 <= $keysym && $keysym <= 122) {
$input->{text} .= chr($keysym);
} elsif ($self->XKeysymToString($keysym) eq 'Return') {
$input->{number} .= '$';
} elsif ($keysym == 59) {
$input->{action} = 'yank';
return 1;
} else {
$escape = 1;
}

if (!$escape) {
my $min = 0;
my $max = scalar(@{$input->{array}});

my $nr_re = qr/^$input->{number}/;
my $text_re = qr/$input->{text}/;
my @matches;
for (my $i = $min; $i < $max; $i++) {
my $item = $input->{array}->[$i];
if ($i =~ /$nr_re/ && $item->{text} =~ /$text_re/) {
push @matches, $i;
} elsif ($input->{unmatch}) {
my $item = $input->{array}->[$i];
$input->{unmatch}->($self, $item);
}
}
if (@matches > 1) {
# not enough data yet
return 1;
}
if (@matches == 1) {
my $item = $input->{array}->[$matches[0]];
my $action = $input->{action} ||
 $item->{action} ||
 'activate';
if ($action eq 'activate') {
$self->exec_async(qw(webview), $item->{text});
} elsif ($action eq 'yank') {
$self->selection($item->{text});
$self->selection_grab(urxvt::CurrentTime);
}
}
}

delete $self->{input};
$self->disable('key_press');
1;
}

sub enable_select {
my $self = shift;
$self->{input} = {text => '', number => '', @_};
$self->enable(key_press => \_select_key);
}

sub matchlist {
my $self = shift;

my @items = $self->find_matches(@_)
or return;

my $max_width = 0;
for (my $i = 0; $i < @items; $i++) {
my $item = $items[$i];

$item->{number} = $i;
$item->{menu} = "$i. $item->{text}";

my $width = $self->strwidth($item->{menu});
$max_width = $width if $width > $max_width;
}

my $rend = urxvt::SET_COLOR(urxvt::OVERLAY_RSTYLE, 0, 3);

my $o = $self->overlay(0, 0, $max_width, scalar(@items), $rend, 0);
for (my $i = 0; $i < @items; $i++) {
$o->set(0, $i, $items[$i]->{menu});
}
$self->enable_select(
array => \@items,
overlay => $o,
unmatch => sub {
my ($self, $item) = @_;
my $o = $self->{input}->{overlay};
$o->set(0, $item->{number}, ' ' x 

Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

[Jeff King]

On Thu, Jan 19, 2017 at 11:30:23AM -0800, Stefan Beller wrote:

> Now let's ask the same question for "git -C sub status ." (which is a
> command that is only reading and not writing to the repository)
> 
> 1) If the submodule is populated, the user clearly intended to know
>more about the submodules status
> 2) It is unclear if the user wanted to learn about the submodules state
>(So ideally: "The submodule 'sub' is not initialized. To init ...")
>or the status check should be applied to the superproject instead.
> 
> Avoid the confusion in 2) as well and just error out for now. Later on
> we may want to add another flag to git.c to allow commands to be run
> inside unpopulated submodules and each command reacts appropriately.

I like the general idea of catching commands in unpopulated submodules,
but I'm somewhat uncomfortable with putting an unconditional check into
git.c, for two reasons:

  1. Reading the index can be expensive. You would not want "git
 rev-parse" to incur this cost.

  2. How does this interact with commands which do interact with the
 index? Don't they expect to find the_index unpopulated?

 (I notice that it's effectively tied to RUN_SETUP, which is good.
  But that also means that many commands, like "diff", won't get the
  benefit. Not to mention non-builtins).

I'd rather see it in the commands themselves. Especially given the
"ideal" in your status example, which requires command-specific
knowledge.

-Peff

Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard

[Jacob Keller]

On Fri, Jan 20, 2017 at 8:09 AM, Jeff King  wrote:
> On Fri, Jan 20, 2017 at 05:22:49PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
>> OK This patch is horrible. Though the idea is cool and I've found it
>> very useful. So here it is. Perhaps the idea may be revised a bit
>> that's more suitable for more than one user.
>>
>> The problem is old, SHA-1 name is not keyboard-friendly, even in
>> abbreviated form. And recent change has made abbrev form longer,
>> harder to type. Most of the time I just go with copy/paste with the
>> mouse, which I don't like. name-rev helps a bit, but it's still long
>> to type (especially with all the ^ and ~ that requires holding shift
>> down).
>
> Not really a comment on your patch itself, but I think a lot of people
> solve this at a higher level, either in their terminal or via a tool
> like tmux.
>
> I recently taught urxvt to recognize sha1s and grab them via keyboard
> hints, and I'm finding it quite useful. Here's what it looks like if
> you're interested:
>
>   http://peff.net/git-hints.gif
>
> The hints technique is taken from pentadactyl (which I also use), but
> the urxvt port is mine. I'm happy to share the code.
>
> Which isn't to say solving it inside Git is wrong, but I've found it
> really convenient for two reasons:
>
>   1. It works whenever you see a sha1, not just in git commands (so
>  emails, inside commit messages, etc).
>
>   2. It doesn't take any screen space until you're ready to select.
>
> The big downside is that it's scraping the screen, so you're guessing at
> what is a sha1. False positives are a little annoying, but usually not
> that big a deal because you're already looking at what you want to
> select, and the hint pops up right there.
>
> -Peff

I would be interested in the code for this.. I'm curious if I can
adapt it to my use of tmux.

Thanks,
Jake

Re: [PATCH] show-ref: Allow --head to work with --verify

[Junio C Hamano]

Vladimir Panteleev  writes:

> This patch adds --head support to show-ref's --verify logic, by
> explicitly checking if the "HEAD" ref is specified when --head is
> present.

> @@ -207,6 +207,8 @@ int cmd_show_ref(int argc, const char **argv, const char 
> *prefix)
>   if (!quiet)
>   show_one(*pattern, );
>   }
> + else if (show_head && !strcmp(*pattern, "HEAD"))
> + head_ref(show_ref, NULL);
>   else if (!quiet)
>   die("'%s' - not a valid ref", *pattern);
>   else

The context around here look like this:

while (*pattern) {
struct object_id oid;

if (starts_with(*pattern, "refs/") &&
!read_ref(*pattern, oid.hash)) {
if (!quiet)
show_one(*pattern, );
}
else if (!quiet)
die("'%s' - not a valid ref", *pattern);
else
return 1;
pattern++;
}

and viewed in the wider context, I notice that quiet is not honored
in the added code.  I think that is easily fixable by replacing this
hunk with something like:

-   if (starts_with(*pattern, "refs/") &&
+   if (to_show_ref(*pattern) &&

and then another hunk that implements to_show_ref() helper function,
perhaps like

static int to_show_ref(const char *pattern)
{
if (starts_with(pattern, "refs/"))
return 1;
if (show_head && !strcmp(pattern, "HEAD"))
return 1;
return 0;
}

or something.

Having said all that, using --verify on HEAD does not make much
sense, because if HEAD is missing in .git/, I do not think Git
considers that directory as a Git repository to begin with.  So from
that point of view, I am not sure what value this change adds to the
system, even though the change is almost correct (modulo the "quiet"
thing).

Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard

[Junio C Hamano]

Jeff King  writes:

> Not really a comment on your patch itself, but I think a lot of people
> solve this at a higher level, either in their terminal or via a tool
> like tmux.
>
> I recently taught urxvt to recognize sha1s and grab them via keyboard
> hints, and I'm finding it quite useful. Here's what it looks like if
> you're interested:
>
>   http://peff.net/git-hints.gif

Nice.  I would have called the "give me the string that is already
on the screen" solution solving at a lower level, not higher, but I
think I agree with the general direction.  I always work in "screen"
and grab a string I see displayed by going into its "copy" mode,
which lets me jump to where the string appears by searching, and I
think that is a solution in the same class.

>   2. It doesn't take any screen space until you're ready to select.

Yup, personally I find this quite important (as I am often on a box
with a smaller screen).

I dream an integration with the command line completion we have.  I
do not offhand see what the mecanism to tell what object names were
shown on the display recently to the completion mechanism should be,
but if we can solve that small detail, the result would be wonderful
;-)

Re: [RFC 1/2] grep: only add delimiter if there isn't one already

[Junio C Hamano]

Stefan Hajnoczi  writes:

> v2.9.3::Makefile may convey that the user originally provided v2.9.3:
> but is that actually useful information?

You are either asking a wrong question, or asking a wrong person
(i.e. Git) the question.  The real question is why the user added a
colon at the end, when "v2.9.3" alone would have sufficed.  What did
the user want to get out of giving an extra colon like "v2.9.3:"?

One answer I can think of is that it is a degenerate case of [2/2],
i.e. if "v2.9.3:t" were an appropriate way to look in the subtree
"t" of "v2.9.3", "v2.9.3:" would be the appropriate way to look in
the whole tree of "v2.9.3".

I understand, from your response to my comment in the thread for
[2/2], that the reason why "v2.9.3:t" was used in the example was
because you felt uncomfortable with using pathspec.  

That's superstition.

My only piece of advice to folks who feel that way is to learn Git
more and get comfortable.  You can do neat things like

   $ git grep -e pattern rev -- t ':!t/helper/'

that you cannot do with "rev:t", for example ;-)

All examples we saw so far are the ones that the user used the colon
syntax when it is more natural to express the command without it.  I
am hesitant to see the behaviour of the command changed to appease
such suboptimal use cases if it requires us to discard a bit of
information, when we haven't established it is OK to lose.

There may be a case

 (1) where the colon syntax is the most natural thing to use, AND
 script reading the output that used that syntax is forced to do
 unnecessary work because "git grep" parrots the colon
 literally, instread of being more intelligent about it
 (i.e. omitting or substituting with slash when the input is a
 tree object, not a tree-ish, as discussed in the thread).

 (2) where the colon syntax is the most natural thing, AND script
 reading the output WANTS to see the distinction in the input
 (remember, "git grep" can take more than one input tree).

We haven't seen either of the above two in the discussion, so the
discussion so far is not sufficient to support the change being
proposed in this RFC, which requires that it is safe to assume that
(1) is the only case where the input is a raw tree (or the input
contains a colon) and (2) will never happen.

So I am still mildly negative on the whole thing.

Re: merge maintaining history

[Junio C Hamano]

Jakub Narębski  writes:

> Then you would have the above history in repositories that fetched
> refs/replace/*, and the one below if replacement info is absent:
>
>original A<-B<-C<-D<-E<-F<---M
> \  /
>first branch  b<-c<-d<-e   /
>  /
>new repo   e*<-f->g->h
>
> But as Junio said it is highly unlikely that you are in this situation.

I do not think I said it is highly unlikely.  I just said that I
didn't read in what David wrote that he did some unusual things, so
I based my conclusion on that assumption.  People who bring problems
here sometimes forget to tell crucial details, and that missing
piece of information often changes how the situation should be
handled.

[PATCH v2 0/2] grep: make output consistent with revision syntax

[Stefan Hajnoczi]

v2:
 * Use obj->type instead of re-parsing name for delimiter
   (Followed Brandon's suggestion but named the variable 'delim' since that
   name is used in other places and 'del' is used for deletion.)
 * Add tests
 * Update Patch 1 commit description with a more relevant example
 * PATCH instead of RFC, now works with all documented git-rev-parse(1) syntax

git-grep(1)'s output is not consistent with git-rev-parse(1) revision syntax.

This means you cannot take "rev:path/to/file.c: foo();" output from git-grep(1)
and expect "git show rev:path/to/file.c" to work.  See the individual patches
for examples of command-lines that produce invalid output.

Stefan Hajnoczi (2):
  grep: only add delimiter if there isn't one already
  grep: use '/' delimiter for paths

 builtin/grep.c  |  8 +++-
 t/t7810-grep.sh | 26 ++
 2 files changed, 33 insertions(+), 1 deletion(-)

-- 
2.9.3

[PATCH v2 1/2] grep: only add delimiter if there isn't one already

[Stefan Hajnoczi]

git-grep(1) output does not follow git's own syntax:

  $ git grep malloc v2.9.3:t/
  v2.9.3:t/:test-lib.sh:  setup_malloc_check () {
  $ git show v2.9.3:t/:test-lib.sh
  fatal: Path 't/:test-lib.sh' does not exist in 'v2.9.3'

This patch avoids emitting the unnecessary ':' delimiter if the name
already ends with ':' or '/':

  $ git grep malloc v2.9.3:
  v2.9.3:t/test-lib.sh:   setup_malloc_check () {
  $ git show v2.9.3:t/test-lib.sh
  (succeeds)

Signed-off-by: Stefan Hajnoczi 
---
 builtin/grep.c  |  6 +-
 t/t7810-grep.sh | 21 +
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 8887b6a..90a4f3d 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -491,7 +491,11 @@ static int grep_object(struct grep_opt *opt, const struct 
pathspec *pathspec,
strbuf_init(, PATH_MAX + len + 1);
if (len) {
strbuf_add(, name, len);
-   strbuf_addch(, ':');
+
+   /* Add a delimiter if there isn't one already */
+   if (name[len - 1] != '/' && name[len - 1] != ':') {
+   strbuf_addch(, ':');
+   }
}
init_tree_desc(, data, size);
hit = grep_tree(opt, pathspec, , , base.len,
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index de2405c..e804a3f 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1435,4 +1435,25 @@ test_expect_success 'grep does not report i-t-a and 
assume unchanged with -L' '
test_cmp expected actual
 '
 
+cat >expected <: for HEAD:t/' '
+   git grep vvv HEAD:t/ >actual &&
+   test_cmp expected actual
+'
+
+cat >expected <: for HEAD:' '
+   git grep vvv HEAD: >actual &&
+   test_cmp expected actual
+'
+
 test_done
-- 
2.9.3

[PATCH v2 2/2] grep: use '/' delimiter for paths

[Stefan Hajnoczi]

If the tree contains a sub-directory then git-grep(1) output contains a
colon character instead of a path separator:

  $ git grep malloc v2.9.3:t
  v2.9.3:t:test-lib.sh: setup_malloc_check () {
  $ git show v2.9.3:t:test-lib.sh
  fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3'

This patch attempts to use the correct delimiter:

  $ git grep malloc v2.9.3:t
  v2.9.3:t/test-lib.sh: setup_malloc_check () {
  $ git show v2.9.3:t/test-lib.sh
  (success)

Signed-off-by: Stefan Hajnoczi 
---
 builtin/grep.c  | 4 +++-
 t/t7810-grep.sh | 5 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 90a4f3d..7a7aab9 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -494,7 +494,9 @@ static int grep_object(struct grep_opt *opt, const struct 
pathspec *pathspec,
 
/* Add a delimiter if there isn't one already */
if (name[len - 1] != '/' && name[len - 1] != ':') {
-   strbuf_addch(, ':');
+   /* rev: or rev:path/ */
+   char delim = obj->type == OBJ_COMMIT ? ':' : 
'/';
+   strbuf_addch(, delim);
}
}
init_tree_desc(, data, size);
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index e804a3f..8a58d5e 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1445,6 +1445,11 @@ test_expect_success 'grep outputs valid : for 
HEAD:t/' '
test_cmp expected actual
 '
 
+test_expect_success 'grep outputs valid : for HEAD:t' '
+   git grep vvv HEAD:t >actual &&
+   test_cmp expected actual
+'
+
 cat >expected <

[PATCH] show-ref: Allow --head to work with --verify

[Vladimir Panteleev]

Previously, when --verify was specified, --head was being ignored, and
"show-ref --verify --head HEAD" would always print "fatal: 'HEAD' -
not a valid ref". As such, when using show-ref to look up any
ref (including HEAD) precisely by name, one would have to special-case
looking up the HEAD ref.

This patch adds --head support to show-ref's --verify logic, by
explicitly checking if the "HEAD" ref is specified when --head is
present.

Signed-off-by: Vladimir Panteleev 
---
 builtin/show-ref.c  | 2 ++
 t/t1403-show-ref.sh | 8 
 2 files changed, 10 insertions(+)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 6d4e66900..ee5078604 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -207,6 +207,8 @@ int cmd_show_ref(int argc, const char **argv, const char 
*prefix)
if (!quiet)
show_one(*pattern, );
}
+   else if (show_head && !strcmp(*pattern, "HEAD"))
+   head_ref(show_ref, NULL);
else if (!quiet)
die("'%s' - not a valid ref", *pattern);
else
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 7e10bcfe3..de64ebfb7 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -164,4 +164,12 @@ test_expect_success 'show-ref --heads, --tags, --head, 
pattern' '
test_cmp expect actual
 '
 
+test_expect_success 'show-ref --verify --head' '
+   {
+   echo $(git rev-parse HEAD) HEAD
+   } >expect &&
+   git show-ref --verify --head HEAD >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.11.0

Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard

[Jeff King]

On Fri, Jan 20, 2017 at 05:22:49PM +0700, Nguyễn Thái Ngọc Duy wrote:

> OK This patch is horrible. Though the idea is cool and I've found it
> very useful. So here it is. Perhaps the idea may be revised a bit
> that's more suitable for more than one user.
> 
> The problem is old, SHA-1 name is not keyboard-friendly, even in
> abbreviated form. And recent change has made abbrev form longer,
> harder to type. Most of the time I just go with copy/paste with the
> mouse, which I don't like. name-rev helps a bit, but it's still long
> to type (especially with all the ^ and ~ that requires holding shift
> down).

Not really a comment on your patch itself, but I think a lot of people
solve this at a higher level, either in their terminal or via a tool
like tmux.

I recently taught urxvt to recognize sha1s and grab them via keyboard
hints, and I'm finding it quite useful. Here's what it looks like if
you're interested:

  http://peff.net/git-hints.gif

The hints technique is taken from pentadactyl (which I also use), but
the urxvt port is mine. I'm happy to share the code.

Which isn't to say solving it inside Git is wrong, but I've found it
really convenient for two reasons:

  1. It works whenever you see a sha1, not just in git commands (so
 emails, inside commit messages, etc).

  2. It doesn't take any screen space until you're ready to select.

The big downside is that it's scraping the screen, so you're guessing at
what is a sha1. False positives are a little annoying, but usually not
that big a deal because you're already looking at what you want to
select, and the hint pops up right there.

-Peff

Re: fatal: bad revision 'git rm -r --ignore-unmatch -- folder'

[jean-christophe manciot]

I've finally found the correct command after some significant research:
git filter-branch --tag-name-filter cat --index-filter "git rm -r
--cached --ignore-unmatch ${file_path}/${file_name}" --prune-empty
--force -- --all

On Fri, Jan 20, 2017 at 4:23 PM, jean-christophe manciot
 wrote:
> I've finally found the correct command after some significant research:
> git filter-branch --tag-name-filter cat --index-filter "git rm -r --cached
> --ignore-unmatch ${file_path}/${file_name}" --prune-empty --force -- --all
>
> On Thu, Jan 19, 2017 at 9:42 AM, jean-christophe manciot
>  wrote:
>>
>> Also some context information:
>> Ubuntu 16.10 4.8
>> git 2.11.0
>>
>> On Thu, Jan 19, 2017 at 9:32 AM, jean-christophe manciot
>>  wrote:
>> > In case you were wondering whether these files were tracked or not:
>> >
>> > git-Games# git ls-files Ubuntu/16.04
>> > Ubuntu/16.04/residualvm_0.3.0~git-1_amd64.deb
>> > Ubuntu/16.04/scummvm-data_1.8.0_all.deb
>> > Ubuntu/16.04/scummvm-data_1.9.0_all.deb
>> > Ubuntu/16.04/scummvm-dbgsym_1.8.2~git20160821.bad86050_amd64.deb
>> > Ubuntu/16.04/scummvm-dbgsym_1.9.0_amd64.deb
>> > Ubuntu/16.04/scummvm_1.8.0_amd64.deb
>> > Ubuntu/16.04/scummvm_1.9.0_amd64.deb
>> >
>> > On Tue, Jan 17, 2017 at 4:30 PM, jean-christophe manciot
>> >  wrote:
>> >> Hi there,
>> >>
>> >> I'm trying to purge a complete folder and its files from the
>> >> repository history with:
>> >>
>> >> git-Games# git filter-branch 'git rm -r --ignore-unmatch --
>> >> Ubuntu/16.04/' --tag-name-filter cat -- --all HEAD
>> >> fatal: bad revision 'git rm -r --ignore-unmatch -- Ubuntu/16.04/'
>> >>
>> >> git does not find the folder although it's there:
>> >>
>> >> git-Games# ll Ubuntu/16.04/
>> >> total 150316
>> >> drwxr-x--- 2 actionmystique actionmystique 4096 Nov 19 13:40 ./
>> >> drwxr-x--- 4 actionmystique actionmystique 4096 Oct 30 14:02 ../
>> >> -rwxr-x--- 1 actionmystique actionmystique  2190394 May  9  2016
>> >> residualvm_0.3.0~git-1_amd64.deb*
>> >> ...
>> >> -rw-r--r-- 1 actionmystique actionmystique 67382492 Oct 13 09:15
>> >> scummvm-dbgsym_1.9.0_amd64.deb
>> >>
>> >> What is going on?
>> >>
>> >> --
>> >> Jean-Christophe
>> >
>> >
>> >
>> > --
>> > Jean-Christophe
>>
>>
>>
>> --
>> Jean-Christophe
>
>
>
>
> --
> Jean-Christophe



-- 
Jean-Christophe

Re: [RFC] stash --continue

[Johannes Schindelin]

Hi Marc,

On Fri, 20 Jan 2017, Marc Branchaud wrote:

> On 2017-01-19 04:30 PM, Johannes Schindelin wrote:
> >
> > At this point I will stop commenting on this issue, as I have said all
> > that I wanted to say about it, at least once. If I failed to get my
> > points across so far, I simply won't be understood.
> 
> Yes, we're obviously looking at this from completely different
> perspectives.

Yes, but you claim to argue from the users' perspective, while I actually
work with enough users to be really certain that I described their mental
model of what an operation is very accurately.

> Stephan (or whoever) if you decide to do this work, I will be content
> with whichever way you choose to go.

So you only wanted to argue and not actually do anything? Tsk, tsk...

:-)

Ciao,
Johannes

Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard

[Johannes Schindelin]

Hi Duy,

On Fri, 20 Jan 2017, Duy Nguyen wrote:

> On Fri, Jan 20, 2017 at 5:46 PM, Johannes Schindelin
>  wrote:
> > Why not introduce a flag to "git log" that shows a keyboard-friendly name
> > similar to what `git name-rev` would have said, except that the name would
> > be generated using the name(s) specified on the command-line?
> >
> > Example:
> >
> > git log 8923d2d0 upstream/pu
> >
> > commit 8923d2d00192ceb1107078484cccf537cb51c1b5 (8923d2d0)
> > ...
> > commit 9f500d6cf5eaa49391d6deca85fc864e5bd23415 (8923d2d0^)
> > ...
> > commit f79c24a291a58845b08cfec7573e22cc153693e1 (8923d2d0~2)
> > ...
> > commit c921c5bb63baaa16dc760de9549da55c8c89dc9c (upstream/pu)
> > ...
> > commit 16793ba6b6333ba0cdee1adb53d979c3fbdb17bc (upstream/pu^)
> > ...
> >
> > Granted, this is still a little more cumbersome to type than @h1, but
> > then, you can skip those round-robin games as well as the possibly
> > backwards-incompatible extension of the rev syntax.
> 
> I mentioned name-rev a few paragraphs down.

Okay, then.

> No, I want the sweet and short @h1 (or something like that). name-rev
> does not qualify.

I am afraid that you have to go back to the drawing board, then, to come
up with a backwards-compatible syntax that is as equally short and sweet
as @h1.

And you will probably also have to come up with a strategy for
implementing "transient refs", because that is exactly what you are doing
here. After all, you need to ensure that @h1 still works correctly (i.e.
without scary warnings) when the corresponding ref has been removed and an
aggressive garbage collection was completed.

And there are other concerns to be considered, not only the
backwards-compatibility. Certainly the current design will need to be
overhauled.

Just from a cursory brain-storming on my side (I even left out the
hand-waving concerns):

- this feature requires write-access. From a user's point of view, this
  should be read-only, or at least also work without write access

- the term "commit mark" is already used for something very different in
  fast-import

- the idea that running "git log --mark-commits master" twice will
  generate *different* marks is surprising (and not in a good way)

- introducing a feature to save keyboard strokes, and then requiring the
  user to type ` --mark-commits`, i.e. 15 key presses, is, uhm, funny

- the fact that these marks are per-worktree makes it a lot less useful
  than if they were per-repository

- related: there is no way to re-use these marks in different
  repositories, e.g. when helping other developers, say, in a chat room

- the fact that the first mark in every repository will be @a1 opens the
  door very wide for using that mark accidentally in the wrong repository.
  At least if I write 8923d in the git-gui repository, it will tell me
  that there is no such revision. Using @a1, it would succeed, and it
  would take me a *long* time to even realize that I am in the wrong
  directory!

And then there are the implementation issues:

- it uses symlinks. I cannot let you do that, Dave.

- using sscanf() to get the value of the first byte of a buffer? What's
  wrong with `*buf*`?

- "asdfghjk"? That is *very* limiting. The opposite of diverse. Just ask
  the French, for starters

- latest_fd is never closed, nor flushed

It sure complicates the implementation side as well as the overall design
a heck of a lot to insist on not using existing rev syntax.

> I don't feel comfortable typing 8923d2d0 without looking at the
> keyboard, and that's a lot of movement on the keyboard.

I mentioned 8923d2d0 only as an example to illustrate the difference to
name-rev.

And when you talk about movement on the keyboard, you *must* realize that
you impose your favorite keyboard layout here. On the German keyboard, for
example, the `@` sign is typed via AltGr+q. That is very cumbersome right
there.

And when you talk about speed advantages of keyboard vs mouse, it seems
that there are studies that show that keyboard is faster than mouse. For
keyboard shortcuts. Not for typing. I actually found no study comparing
the speed of typing with the speed of copy/paste, although I am sure that
they exist, and that they show pretty clearly that copy/paste is faster,
and more accurate, than typing.

But back to minimizing movement on the keyboard.

You would usually type `git log upstream/pu`, tab-completing
`upstream/pu`, right? And then you would want to refer to one particular
entry in that output, say, `upstream/pu~50^2`, right?

Right:

> upstream/pu is a bit better, but still very long (at least for me). Yes
> TAB-ing does help, but not enough. Then you'll get the dreadful "^2~1^3"
> dance.

Yes, exactly. You would use tab-completion there, too, most of the way.
Although I do not find that "^2^^3" [*1*] dreadful, I can see that some
users find it cumbersome. And then it is a little bit over 

Re: [RFC] stash --continue

[Marc Branchaud]

On 2017-01-19 04:30 PM, Johannes Schindelin wrote:


At this point I will stop commenting on this issue, as I have said all
that I wanted to say about it, at least once. If I failed to get my points
across so far, I simply won't be understood.


Yes, we're obviously looking at this from completely different perspectives.

Stephan (or whoever) if you decide to do this work, I will be content 
with whichever way you choose to go.


M.

ATTANTION SIR/MADAM

[aisha.gad...@ono.com]

Am Dr. Aisha Gaddafi, the daughter of late Libyan president, I need a 
partner or an investor that will help me in investing the sum of $87.5 
million USD in his or her country, the funds is deposited here in 
Burkina Faso where I am staying for the moment with my family, I would 
like to know your mind tows the investment  of the total sum mentioned 
above, note you will received  30% of the total sum after the transfer 
is completed into your account, for more details, 
Yours friend Dr. Aisha Gaddafi

Re: [RFC 0/2] grep: make output consistent with revision syntax

[Jeff King]

On Fri, Jan 20, 2017 at 02:18:32PM +, Stefan Hajnoczi wrote:

> > Are there cases you know that aren't covered by your patches?
> 
> From Patch 2/2:
> 
>   This patch does not cope with @{1979-02-26 18:30:00} syntax and treats
>   it as a path because it contains colons.
> 
> If we use obj->type instead of re-parsing the name then that problem is
> solved.

Ah, right. I somehow totally missed that, and blindly assumed your colon
search was only at the end. But of course that wouldn't work at all (it
would miss "v2.9.3:t").

So yes, definitely it should be checking the object type.

-Peff

Re: [PATCH] log: new option decorate reflog of remote refs

[Jeff King]

On Fri, Jan 20, 2017 at 05:55:21PM +0700, Duy Nguyen wrote:

> > We already have very flexible ref-selectors like --remotes, --branches,
> > etc. The generalization of this would perhaps be something like:
> >
> >   git log --decorate-reflog --remotes --branches
> >
> > where "--decorate-reflog" applies to the next ref selector and then is
> > reset, the same way --exclude is. And it includes those refs _only_ for
> > decoration, not for traversal. So you could do:
> >
> >   git log --decorate-reflog --remotes --remotes
> >
> > if you wanted to see use those as traversal roots, too (if this is
> > common, it might even merit another option for "decorate and show").
> >
> > That's just off the top of my head, so maybe there are issues. I was
> > just surprised to see the "-remote" part in your option name.
> 
> Imposing order between options could cause confusion, I think, if you
> remove --decorate-reflog leaving --remotes on by accident, now you get
> --remotes with a new meaning. We could go with something like
> --decodate-reflog=remote, but that clashes with the number of reflog
> entries and we may need a separator, like --decorate-reflog=remote,3.
> Or we could add something to --decorate= in addition to
> short|full|auto|no. Something like --decorate=full,reflog or
> --decorate=full,reflog=remote,entries=3 if I want 3 reflog entries.

I agree that making option-order important is potentially confusing. But
it does already exist with --exclude. It's necessary to specify some
sets of refs (e.g., all of A, except for those that match B, and then
all of C, including those that match B).

Having --decorate-reflog=remote would be similarly constrained. You
couldn't do "decorate all remotes except for these ones". For that
matter, I'm not sure how you would do "decorate just the refs from
origin".

I'll grant that those are going to be a lot less common than just "all
the remotes" (or all the tags, or whatever). I'd just hate to see us
revisiting this in a year to generalize it, and being stuck with
historical baggage.

> My hesitant to go that far is because I suspect decorating reflog
> won't be helpful for non-remotes. But I'm willing to make more changes
> if it opens door to master.

Forgetting reflogs for a moment, I'd actually find it useful to just
decorate tags and local branches, but not remotes. But right now there
isn't any way to select which refs are worthy of decoration (reflog or
not).

That's why I'm thinking so much about a general ref-selection system. I
agree the "--exclude=... --remotes" thing is complicated, but it's also
the ref-selection system we _already_ have, which to me is a slight
point in its favor.

-Peff

Re: [RFC 2/2] grep: use '/' delimiter for paths

[Jeff King]

On Fri, Jan 20, 2017 at 02:12:12PM +, Stefan Hajnoczi wrote:

> I find : vs  --  confusing:
> 
> | : |  -- 
>   --+--+-
>   git grep  | OK   | OK
>   --+--+-
>   git show  | OK   |  ignored
>   --+--+-
>   git log   | no output| OK
>   --+--+-
> 
> Neither syntax always does what I expect.  If git show  -- 
> honored  then I could use that syntax consistently.
> 
> Sorry for going on a tangent.  Does it seem reasonable to handle 
> in git-show(1) as a UI convenience?

It's not ignored; just as with git-log, it's a pathspec to limit the
diff. E.g.:

  $ git show --name-status v2.9.3
  ...
  M   Documentation/RelNotes/2.9.3.txt
  M   Documentation/git.txt
  M   GIT-VERSION-GEN

  $ git show --name-status v2.9.3 -- Documentation
  M   Documentation/RelNotes/2.9.3.txt
  M   Documentation/git.txt

That's typically less useful than it is with log (where limiting the
diff also kicks in history simplification and omits some commits
entirely). But it does do something.

-Peff

Re: [RFC 0/2] grep: make output consistent with revision syntax

[Stefan Hajnoczi]

On Thu, Jan 19, 2017 at 11:59:59AM -0500, Jeff King wrote:
> On Thu, Jan 19, 2017 at 03:03:45PM +, Stefan Hajnoczi wrote:
> 
> > git-grep(1)'s output is not consistent with git-rev-parse(1) revision 
> > syntax.
> > 
> > This means you cannot take "rev:path/to/file.c: foo();" output from 
> > git-grep(1)
> > and expect "git show rev:path/to/file.c" to work.  See the individual 
> > patches
> > for examples of command-lines that produce invalid output.
> 
> I think this is a good goal.
> 
> I couldn't immediately think of any cases where your patches would
> misbehave, but my initial thought was that the "/" versus ":"
> distinction is about whether the initial object is a tree or a commit.
> 
> You do still have to special case the root tree (so "v2.9.3:" does not
> get any delimiter). I think "ends in a colon" is actually a reasonable
> way of determining that.
> 
> > This series is an incomplete attempt at solving the issue.  I'm not familiar
> > enough with the git codebase to propose a better solution.  Perhaps someone 
> > is
> > interested in a proper fix?
> 
> Are there cases you know that aren't covered by your patches?

From Patch 2/2:

  This patch does not cope with @{1979-02-26 18:30:00} syntax and treats
  it as a path because it contains colons.

If we use obj->type instead of re-parsing the name then that problem is
solved.


signature.asc
Description: PGP signature

Re: [RFC 2/2] grep: use '/' delimiter for paths

[Stefan Hajnoczi]

On Thu, Jan 19, 2017 at 10:29:34AM -0800, Brandon Williams wrote:
> On 01/19, Stefan Hajnoczi wrote:
> > If the tree contains a sub-directory then git-grep(1) output contains a
> > colon character instead of a path separator:
> > 
> >   $ git grep malloc v2.9.3:t
> >   v2.9.3:t:test-lib.sh: setup_malloc_check () {
> >   $ git show v2.9.3:t:test-lib.sh
> >   fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3'
> > 
> > This patch attempts to use the correct delimiter:
> > 
> >   $ git grep malloc v2.9.3:t
> >   v2.9.3:t/test-lib.sh: setup_malloc_check () {
> >   $ git show v2.9.3:t/test-lib.sh
> >   (success)
> > 
> > This patch does not cope with @{1979-02-26 18:30:00} syntax and treats
> > it as a path because it contains colons.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  builtin/grep.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/builtin/grep.c b/builtin/grep.c
> > index 3643d8a..06f8b47 100644
> > --- a/builtin/grep.c
> > +++ b/builtin/grep.c
> > @@ -493,7 +493,8 @@ static int grep_object(struct grep_opt *opt, const 
> > struct pathspec *pathspec,
> >  
> > /* Add a delimiter if there isn't one already */
> > if (name[len - 1] != '/' && name[len - 1] != ':') {
> > -   strbuf_addch(, ':');
> > +   /* rev: or rev:path/ */
> > +   strbuf_addch(, strchr(name, ':') ? '/' : 
> > ':');
> 
> As Jeff mentioned it may be better to base which character gets appended
> by checking the obj->type field like this maybe:
> 
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 69dab5dc5..9dfe11dc7 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -495,7 +495,8 @@ static int grep_object(struct grep_opt *opt, const struct 
> pathspec *pathspec,
>   /* Add a delimiter if there isn't one already */
>   if (name[len - 1] != '/' && name[len - 1] != ':') {
>   /* rev: or rev:path/ */
> - strbuf_addch(, strchr(name, ':') ? '/' : 
> ':');
> + char del = obj->type == OBJ_COMMIT ? ':' : '/';
> + strbuf_addch(, del);

Nice, that also solves the false positive with @{1979-02-26 18:30:00}.

Stefan


signature.asc
Description: PGP signature

Re: [RFC 2/2] grep: use '/' delimiter for paths

[Stefan Hajnoczi]

On Thu, Jan 19, 2017 at 10:54:02AM -0800, Junio C Hamano wrote:
> Stefan Hajnoczi  writes:
> 
> > If the tree contains a sub-directory then git-grep(1) output contains a
> > colon character instead of a path separator:
> >
> >   $ git grep malloc v2.9.3:t
> >   v2.9.3:t:test-lib.sh: setup_malloc_check () {
> >   $ git show v2.9.3:t:test-lib.sh
> >   fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3'
> 
> I am slightly less negative on this compared to 1/2, but not by a
> big margin.  The user wanted to feed a subtree to the command,
> instead of doing the more natural
> 
> $ git grep -e malloc v2.9.3 -- t

I find : vs  --  confusing:

| : |  -- 
  --+--+-
  git grep  | OK   | OK
  --+--+-
  git show  | OK   |  ignored
  --+--+-
  git log   | no output| OK
  --+--+-

Neither syntax always does what I expect.  If git show  -- 
honored  then I could use that syntax consistently.

Sorry for going on a tangent.  Does it seem reasonable to handle 
in git-show(1) as a UI convenience?

> So again, "contains a colon character" is not coming from what Git
> does, but the user gave Git "a colon character" when she didn't have
> to.
> 
> Having said that, if we wanted to start ignoring what the end-user
> said in the initial input like 1/2 and 2/2 does (i.e. "this specific
> tree object" as an input), I think the approach to solve for 1/2 and
> 2/2 should be the same.  I think we should decide to do a slash
> instead of a colon, not because v2.9.3: has colon at the end and
> v2.9.3:t has colon in it, but because both of these are both bare
> tree objects.  The patches presented does not seem to base their
> decision on the actual object type but on the textual input, which
> seems wrong.

Yes, reparsing the name is ugly and I hoped to get feedback with this
RFC.  Thanks for the quick review!


signature.asc
Description: PGP signature

Re: [RFC 1/2] grep: only add delimiter if there isn't one already

[Stefan Hajnoczi]

On Thu, Jan 19, 2017 at 10:39:02AM -0800, Junio C Hamano wrote:
> Stefan Hajnoczi  writes:
> 
> > git-grep(1) output does not follow git's own syntax:
> >
> >   $ git grep malloc v2.9.3:
> >   v2.9.3::Makefile:   COMPAT_CFLAGS += -Icompat/nedmalloc
> >   $ git show v2.9.3::Makefile
> >   fatal: Path ':Makefile' does not exist in 'v2.9.3'
> >
> > This patch avoids emitting the unnecessary ':' delimiter if the name
> > already ends with ':' or '/':
> >
> >   $ git grep malloc v2.9.3:
> >   v2.9.3:Makefile:   COMPAT_CFLAGS += -Icompat/nedmalloc
> >   $ git show v2.9.3:Makefile
> >   (succeeds)
> 
> I am mildly negative on this one.  I suspect that the above example
> is a made-up one and you might have a more compelling real-world use
> case in mind, but at least the above does not convince me.

Another trailing delimiter example:

  $ git grep malloc v2.9.3:t/
  v2.9.3:t/:test-lib.sh:  setup_malloc_check () {

After Patch 1/2:

  v2.9.3:t/test-lib.sh:  setup_malloc_check () {

> The end-user explicitly gave the extra ':' because she wanted to
> feed the tree object, not a tag that leads to the tree, for some
> reason.  I think the output should respect that and show how she
> spelled the starting point.  IOW, it is not "':' added unnecessarily
> by Git".  It is ':' added by the end-user for whatever reason.

v2.9.3::Makefile may convey that the user originally provided v2.9.3:
but is that actually useful information?  I don't see what the user will
do with this information (and they should already know since they
provided the command-line).

v2.9.3:Makefile can be copy-pasted or extracted by scripts for further
git commands.  That is useful.

Stefan


signature.asc
Description: PGP signature

Re: [RFC PATCH 0/5] Localise error headers

[Duy Nguyen]

On Fri, Jan 20, 2017 at 8:23 PM, Duy Nguyen  wrote:
> I've tested it a bit. Seems to work ok.

Oops. Didn't notice that the "fatal:" bit is still untranslated but
Micheal's patch can handle that.
-- 
Duy

Re: [PATCH] git-prompt.sh: add submodule indicator

[SZEDER Gábor]

I'm not well-versed in submodules, so I won't comment on whether this
is the right way to determine that a repository is a submodule, but I
was surprised to see how much you have to work to find this out.

My comments will mostly focus on how to eliminate or at least limit
the scope of subshells and command executions, because fork()s and
exec()s are rather expensive on Windows.

On Fri, Jan 20, 2017 at 1:07 AM, Benjamin Fuchs  wrote:
> I expirienced that working with submodules can be confusing. This indicator
> will make you notice very easy when you switch into a submodule.
> The new prompt will look like this: (sub:master)
> Adding a new optional env variable for the new feature.
>
> Signed-off-by: Benjamin Fuchs 
> ---
>  contrib/completion/git-prompt.sh | 37 -
>  1 file changed, 36 insertions(+), 1 deletion(-)

Tests?

> diff --git a/contrib/completion/git-prompt.sh 
> b/contrib/completion/git-prompt.sh
> index 97eacd7..4c82e7f 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -93,6 +93,10 @@
>  # directory is set up to be ignored by git, then set
>  # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
>  # repository level by setting bash.hideIfPwdIgnored to "false".
> +#
> +# If you would like __git_ps1 to indicate that you are in a submodule,
> +# set GIT_PS1_SHOWSUBMODULE. In this case a "sub:" will be added before
> +# the branch name.
>
>  # check whether printf supports -v
>  __git_printf_supports_v=
> @@ -284,6 +288,32 @@ __git_eread ()
> test -r "$f" && read "$@" <"$f"
>  }
>
> +# __git_is_submodule
> +# Based on:
> +# 
> http://stackoverflow.com/questions/7359204/git-command-line-know-if-in-submodule
> +__git_is_submodule ()

Use the '__git_ps1' prefix in the function name, like the other
functions.

> +{
> +   local git_dir parent_git module_name path
> +   # Find the root of this git repo, then check if its parent dir is 
> also a repo
> +   git_dir="$(git rev-parse --show-toplevel)"

 1. This is a somewhat misleading variable name, because git_dir (with
or without underscore or dash) refers to the path to the .git
directory of a repository through the codebase, documentation and
CLI options, not the top-level directory of the worktree.

 2. In a bare repository or in the .git directory of a "regular"
repository '--show-toplevel' doesn't output anything, leading to
an empty $module_name below, which ultimately results in no
submodule indicator.

As fas as behavior is concerned, this is good in the bare
repository case, because as I understand it there is no such thing
as a bare submodule.  I'm not sure whether the submodule indicator
should be displayed in the ".git dir of a submodule" case.  I
leave it up to you and Stefan to discuss.

However, the info about whether we are in a bare repository or
.git dir is already availabe in certain variables, so we know
upfront when the current repository can't be a submodule.  In
those cases this function should not be called only to spend
several subshells and command executions to figure out what we
already knew anyway.

 3. The path to the .git directory of the current repository
is already available in the (not particularly descriptively named)
$g variable from __git_ps1.  Perhaps you could use that variable
instead, thus avoiding a subshell and executing a git command
here.

> +   module_name="$(basename "$git_dir")"

This is executed even when there is no repository in the parent
directories, but it's only needed when there _is_ a repo up there.
Please move it into the conditional below, to avoid a subshell and
command execution in the main code path.

Since this $git_dir comes directly from our own 'git rev-parse' do we
have to be prepared for a Windows-style path there?  If it were always
a UNIX-style path, then we could strip all directories with shell
parameter expansion, eliminating both the subshell and the basename
execution.

> +   parent_git="$(cd "$git_dir/.." && git rev-parse --show-toplevel 2> 
> /dev/null)"

Style nit: no space after redirection, i.e. it should be '2>/dev/null'.

More importantly, I don't think you really need this variable:

 1. The existence of a parent git repository can be determined from
'git rev-parse's exit code alone.

 2. When you run 'git submodule' below, you don't have to cd to the
_top-level_ directory of the parent repository's worktree.  You
just have to cd to _any_ directory in the parent, and you can do
that with 'git -C "$git_dir/.." submodule ...', without knowing
the parent's top-level directory.

This means that you don't need 'git rev-parse's output, thus there is
no need for the command substitution.  Yet another subshell spared! :)
However, then you have to be careful with changing directories, and
should write it 

Re: [RFC PATCH 0/5] Localise error headers

[Duy Nguyen]

On Wed, Jan 11, 2017 at 06:37:25AM -0500, Jeff King wrote:
> > To find a good example, "git grep die" giving me some food of though:
> > 
> > die_errno(..) should always take a string marked up for translation,
> > because the errno string is translated?
> 
> Yes, I would think die_errno() is a no-brainer for translation, since
> the strerror() will be translated.

I agree. And the main (*) changes are relative simple too. I've tested
it a bit. Seems to work ok.

-- 8< --
diff --git a/Makefile b/Makefile
index d861bd9985..6f88c6cac5 100644
--- a/Makefile
+++ b/Makefile
@@ -2102,7 +2102,7 @@ XGETTEXT_FLAGS = \
--msgid-bugs-address="Git Mailing List " \
--from-code=UTF-8
 XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
-   --keyword=_ --keyword=N_ --keyword="Q_:1,2"
+   --keyword=_ --keyword=N_ --keyword="Q_:1,2" --keyword=die_errno
 XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \
--keyword=gettextln --keyword=eval_gettextln
 XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \
diff --git a/usage.c b/usage.c
index 17f52c1b5c..c022726f9c 100644
--- a/usage.c
+++ b/usage.c
@@ -159,7 +159,7 @@ void NORETURN die_errno(const char *fmt, ...)
}
 
va_start(params, fmt);
-   die_routine(fmt_with_err(buf, sizeof(buf), fmt), params);
+   die_routine(fmt_with_err(buf, sizeof(buf), _(fmt)), params);
va_end(params);
 }
 
-- 8< --

(*) We would need another patch to remove _() from die_errno(_(..)).
But that's something I expect coccinelle to be excel at.
--
Duy

Re: [RFC PATCH 0/5] Localise error headers

[Duy Nguyen]

On Thu, Jan 12, 2017 at 1:08 AM, Junio C Hamano  wrote:
> Jeff King  writes:
>
>> Yes, I would think die_errno() is a no-brainer for translation, since
>> the strerror() will be translated.
>>
>>> apply.c:die(_("internal error"));
>>>
>>> That is funny, too. I think we should substitute that with
>>>
>>> die("BUG: untranslated, but what went wrong instead")
>>
>> Yep. We did not consistently use "BUG:" in the early days. I would say
>> that "BUG" lines do not need to be translated. The point is that nobody
>> should ever see them, so it seems like there is little point in giving
>> extra work to translators.
>
> In addition, "BUG: " is relatively recent introduction to our
> codebase.  Perhaps having a separate BUG() function help the
> distinction further?

I was going to write the same thing. On top of that I wonder if have
enough "if (something) die("BUG:")" to justify stealing BUG_ON() from
kernel (better than assert since the condition will always be
evaluated).
-- 
Duy

How To Write Thesis Proposal in Best Way

[raniya]

Writing a winning  Thesis Proposal
   is considered to be
very difficult task by the students.But now you don't need to be worried
about thesis proposal writing  DissertationHelp.ae
   has helped many
students to finish their thesis research with less stress and confidence.





-
Proposal Writer 


--
View this message in context: 
http://git.661346.n2.nabble.com/How-To-Write-Thesis-Proposal-in-Best-Way-tp7657464.html
Sent from the git mailing list archive at Nabble.com.

Re: merge maintaining history

[Jakub Narębski]

W dniu 19.01.2017 o 22:42, Junio C Hamano pisze:
> "David J. Bakeman"  writes:
 
[...]
>> Thanks I think that's close but it's a little more complicated I think
>> :<(  I don't know if this diagram will work but lets try.
>>
>> original A->B->C->D->E->F
>>  \
>> first branch  b->c->d->e
>>
>> new repo e->f->g->h
>>
>> Now I need to merge h to F without loosing b through h hopefully.  Yes e
>> was never merged back to the original repo and it's essentially gone now
>> so I can't just merge to F or can I?
> 
> With the picture, I think you mean 'b' is forked from 'B' and the
> first branch built 3 more commits on top, leading to 'e'.
> 
> You say "new repo" has 'e' thru 'h', and I take it to mean you
> started developing on top of the history that leads to 'e' you built
> in the first branch, and "new repo" has the resulting history that
> leads to 'h'.
> 
> Unless you did something exotic and non-standard, commit 'e' in "new
> repo" would be exactly the same as 'e' sitting on the tip of the
> "first branch", so the picture would be more like:
> 
>> original A->B->C->D->E->F
>>  \
>> first branch  b->c->d->e
>> \
>> new repo f->g->h
> 
> no?

On the other hand Git has you covered even if you did something 
non-standard, like starting new repo from the _state_ of 'e', that
is you have just copied files and created new repository, having
'e' (or actually 'e*') as an initial commit.

   original A<-B<-C<-D<-E<-F
\
   first branch  b<-c<-d<-e

   new repo   e*<-f<-g<-h

Note that arrows are in reverse direction, as it is newer commit
pointing to its parents, not vice versa.

Assuming that you have everything in a single repository, by adding
both original and new repo as "remotes", you can use 'git replace'
command to replace 'e*' with 'e'.

   original A<-B<-C<-D<-E<-F
\
   first branch  b<-c<-d<-e
   \
   new repo \-f<-g<-h
   (with refs/replace)

> Then merging 'h' into 'F' will pull everything you did since
> you diverged from the history that leads to 'F', resulting in a
> history of this shape:
> 
>> original A->B->C->D->E->F--M
>>  \/
>> first branch  b->c->d->e /
>> \   /
>> new repo f->g->h

Then you would have the above history in repositories that fetched
refs/replace/*, and the one below if replacement info is absent:

   original A<-B<-C<-D<-E<-F<---M
\  /
   first branch  b<-c<-d<-e   /
 /
   new repo   e*<-f->g->h

But as Junio said it is highly unlikely that you are in this situation.

HTH
-- 
Jakub Narębski

Re: The design of refs backends, linked worktrees and submodules

[Duy Nguyen]

On Thu, Jan 19, 2017 at 8:30 PM, Michael Haggerty  wrote:
> I kindof think that it would have been a better design to store the
> references for all linked worktrees in the main repository's ref-store.
> For example, the "bisect" refs for a worktree named "" could have
> been stored under "refs/worktrees//bisect/*". Then either:
>
> * teach the associated tools to read/write references there directly
> (probably with DWIM rules to make command-line use easier), or
> * treat these references as if they were actually at a standard place
> like `refs/worktree/bisect/*`; i.e., users would need to know that they
> were per-worktree references, but wouldn't need to worry about the true
> locations, or
> * treat these references as if they were actually in their traditional
> locations (though it is not obvious how this scheme could be expanded to
> cover new per-worktree references).

Well. In one direction, we store everything at one place and construct
different slices of view of the unified store. On the other far end,
we have plenty of one-purpose stores, then combine them as we need.
It's probably personal taste, but I prefer the latter.

Making a single big store could bring us closer to the "big number"
problem. Yeah we will have to handle million of refs anyway, someday.
That does not mean we're free to increase the number of refs a few
more times. Then there are separate stores by nature like submodules
(caveat: I haven't checked out your submodule-hash branch), or the
problem with multiple repos sharing objects/info/alternates.

> This is a topic that I have thought a lot about. I definitely like this
> direction. In fact I've dabbled around with some first steps; see branch
> `submodule-hash` in my fork on GitHub [1]. That branch associates a
> `ref_store` more closely with the directory where the references are
> stored, as opposed to having a 1:1 relationship between `ref_store`s and
> submodules.

Thanks. Will check it out.

> Let me braindump some more information about this topic.
> ...

Juicy stuff :D It's hard to know these without staring really long and
hard at refs code. Thank you.

> I've taken some stabs at picking these apart into separate ref stores,
> but haven't had time to make very satisfying progress. By the time of
> GitMerge I might have a better feeling for whether I can devote some
> time to this project.

I think sending WIP patches to the list from time to time is also
helpful, even if it's not perfect. For one thing I would know you were
doing (or thinking at least, which also counts) and not stepping on
each other. On my part I'm not attempting to make any more changes (*)
until after I've read your branches.

(*) I took git_path() out of refs code and was surprised that multi
worktree broke. Silly me. Wrong first step.
-- 
Duy

Re: Git: new feature suggestion

[Jakub Narębski]

W dniu 20.01.2017 o 01:26, Linus Torvalds pisze:
> On Thu, Jan 19, 2017 at 1:48 PM, Jakub Narębski  wrote:
>> W dniu 19.01.2017 o 19:39, Linus Torvalds pisze:
>>>
>>> You can do it in tig, but I suspect a more graphical tool might be better.
>>
>> Well, we do have "git gui blame".
> 
> Does that actually work for people? Because it really doesn't for me.
> 
> And I'm not just talking about the aesthetics of the thing, but the
> whole experience, and the whole "dig into parent" which just gives me
> an error message.

Strange. I had been using "git gui blame" _because_ of its "dig to parent"
functionality, and it worked for me just fine.

The other thing that I like about "git gui blame" is that it shows both
the commit that moved the fragment of code (via "git blame"), and the
commit that created the fragment of code (via "git blame -C -C -w", I think).


Anyway, all of this (sub)discussion is about archeology, but what might
be more important is automatic rename handling when integrating changes,
be it git-am, git-merge, or something else...

-- 
Jakub Narębski

Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard

[Duy Nguyen]

On Fri, Jan 20, 2017 at 5:46 PM, Johannes Schindelin
 wrote:
> Why not introduce a flag to "git log" that shows a keyboard-friendly name
> similar to what `git name-rev` would have said, except that the name would
> be generated using the name(s) specified on the command-line?
>
> Example:
>
> git log 8923d2d0 upstream/pu
>
> commit 8923d2d00192ceb1107078484cccf537cb51c1b5 (8923d2d0)
> ...
> commit 9f500d6cf5eaa49391d6deca85fc864e5bd23415 (8923d2d0^)
> ...
> commit f79c24a291a58845b08cfec7573e22cc153693e1 (8923d2d0~2)
> ...
> commit c921c5bb63baaa16dc760de9549da55c8c89dc9c (upstream/pu)
> ...
> commit 16793ba6b6333ba0cdee1adb53d979c3fbdb17bc (upstream/pu^)
> ...
>
> Granted, this is still a little more cumbersome to type than @h1, but
> then, you can skip those round-robin games as well as the possibly
> backwards-incompatible extension of the rev syntax.

I mentioned name-rev a few paragraphs down. No, I want the sweet and
short @h1 (or something like that). name-rev does not qualify. I don't
feel comfortable typing 8923d2d0 without looking at the keyboard, and
that's a lot of movement on the keyboard.  upstream/pu is a bit
better, but still very long (at least for me). Yes TAB-ing does help,
but not enough. Then you'll get the dreadful "^2~1^3" dance.
-- 
Duy

Re: [PATCH] log: new option decorate reflog of remote refs

[Duy Nguyen]

On Fri, Jan 20, 2017 at 12:23 AM, Jeff King  wrote:
> I think it's a neat idea, but the actual option:
>
>> +--decorate-remote-reflog[=]::
>> + Decorate `` most recent reflog entries on remote refs, up
>> + to the specified number of entries. By default, only the most
>> + recent reflog entry is decorated.
>
> seems weirdly limited and non-orthogonal. What happens when somebody
> wants to decorate other reflogs besides refs/remotes?
>
> We already have very flexible ref-selectors like --remotes, --branches,
> etc. The generalization of this would perhaps be something like:
>
>   git log --decorate-reflog --remotes --branches
>
> where "--decorate-reflog" applies to the next ref selector and then is
> reset, the same way --exclude is. And it includes those refs _only_ for
> decoration, not for traversal. So you could do:
>
>   git log --decorate-reflog --remotes --remotes
>
> if you wanted to see use those as traversal roots, too (if this is
> common, it might even merit another option for "decorate and show").
>
> That's just off the top of my head, so maybe there are issues. I was
> just surprised to see the "-remote" part in your option name.

Imposing order between options could cause confusion, I think, if you
remove --decorate-reflog leaving --remotes on by accident, now you get
--remotes with a new meaning. We could go with something like
--decodate-reflog=remote, but that clashes with the number of reflog
entries and we may need a separator, like --decorate-reflog=remote,3.
Or we could add something to --decorate= in addition to
short|full|auto|no. Something like --decorate=full,reflog or
--decorate=full,reflog=remote,entries=3 if I want 3 reflog entries.

My hesitant to go that far is because I suspect decorating reflog
won't be helpful for non-remotes. But I'm willing to make more changes
if it opens door to master.
-- 
Duy

Re: Git: new feature suggestion

[Joao Pinto]

Hi Stefan,

Às 10:03 PM de 1/19/2017, Stefan Beller escreveu:
> On Thu, Jan 19, 2017 at 1:51 PM, Joao Pinto  wrote:
>> Às 7:16 PM de 1/19/2017, Linus Torvalds escreveu:
>>> On Thu, Jan 19, 2017 at 10:54 AM, Joao Pinto  
>>> wrote:

 I am currently facing some challenges in one of Linux subsystems where a 
 rename
 of a set of folders and files would be the perfect scenario for future
 development, but the suggestion is not accepted, not because it's not 
 correct,
 but because it makes the maintainer life harder in backporting bug fixes 
 and new
 features to older kernel versions and because it is not easy to follow the
 renamed file/folder history from the kernel.org history logs.
>>>
>>> Honestly, that's less of a git issue, and more of a "patch will not
>>> apply across versions" issue.
>>>
>>> No amount of rename detection will ever fix that, simply because the
>>> rename hadn't even _happened_ in the old versions that things get
>>> backported to.
>>>
>>> ("git cherry-pick" can do a merge resolution and thus do "backwards"
>>> renaming too, so tooling can definitely help, but it still ends up
>>> meaning that even trivial patches are no longer the _same_ trivial
>>> patch across versions).
>>>
>>> So renaming things increases maintainer workloads in those situations
>>> regardless of any tooling issues.
>>>
>>> (You may also be referring to the mellanox mess, where this issue is
>>> very much exacerbated by having different groups working on the same
>>> thing, and maintainers having very much a "I will not take _anything_
>>> from any of the groups that makes my life more complicated" model,
>>> because those groups fucked up so much in the past).
>>>
>>> In other words, quite often issues are about workflows rather than
>>> tools. The networking layer probably has more of this, because David
>>> actually does the backports himself, so he _really_ doesn't want to
>>> complicate things.
>>
>> I totally understand David' side! Synopsys is a well-known IP Vendor, and 
>> for a
>> long time its focus was the IP only. Knowadays the strategy has changed and
>> Synopsys is very keen to help in Open Source, namelly Linux, developing the
>> drivers for new IP Cores and participating in the improvement of existing 
>> ones.
>> I am part of the team that has that job.
>>
>> In USB and PCI subystems developers created common Synopsys drivers (focused 
>> on
>> the HW IP) and so today they are massively used by all the SoC that use 
>> Synopsys
>> IP.
>>
>> In the network subsystem, there are some drivers that target the same IP but
>> were made by different companies. stmmac is an excelent driver for Synopsys 
>> MAC
>> 10/100/1000/QOS IPs, but there was another driver made by AXIS driver that 
>> also
>> targeted the QOS IP. We detected that issue and merged the AXIS specific 
>> driver
>> ops to stmmac, and nowadays, AXIS uses stmmac. So less drivers to maintain!
>>
>> The idea that was rejected consisted of renaming stmicro/stmmac to dwc/stmmac
>> and to have dwc (designware controllers) as the official driver spot for
>> Synopsys Ethernet IPs.
>> There is another example of duplication, which is AMD' and Samsung' XGMAC
>> driver, targeting the same Synopsys XGMAC IP.
>>
>> I am giving this examples because although the refactor adds work for
>> backporting, it reduces the maintenance since we would have less duplicated
>> drivers as we have today.
> 
> This sounds as if the code in question would only receive backports
> for a specific
> time (determined by HW lifecycle, maintenance life cycle and such).
> 
> So I wonder if this could be solved by not just renaming but
> additionally adding a
> symbolic link, such that the files in question seem to appear twice on
> the file system.
> Then backports ought to be applicable (hoping git-am doesn't choke on 
> symlinks),
> and after a while once the there no backports any more (due to life
> cycle reasons),
> remove the link?
> 
> This also sounds like a kind of problem, that others have run into before,
> how did they solve it?

I am currently involved in the PCI host/ refactor process and that will cause a
total reorganization of the folder, because a new PCIe Endpoint is comming up
from Texas Instruments. Bjorn (PCI Maintainer) is ok with it.
The network subsystem, is a very busy one, with lots of activity, and so I
understand David Miller' point, because he already has work overload, but we
have to find a way to improve it and prepare for the future.

I think this a hot topic, that should be discussed, since it might hold back the
evolution of some subystems.

Thanks,
Joao

> 
> Thanks,
> Stefan
> 
>>
>> Thanks,
>> Joao
>>
>>
>>>Linus
>>>
>>

Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard

[Johannes Schindelin]

Hi Duy,

On Fri, 20 Jan 2017, Nguyễn Thái Ngọc Duy wrote:

> OK This patch is horrible.

Yeah ;-)

> Though the idea is cool and I've found it very useful. So here it is.
> Perhaps the idea may be revised a bit that's more suitable for more than
> one user.

Why not introduce a flag to "git log" that shows a keyboard-friendly name
similar to what `git name-rev` would have said, except that the name would
be generated using the name(s) specified on the command-line?

Example:

git log 8923d2d0 upstream/pu

commit 8923d2d00192ceb1107078484cccf537cb51c1b5 (8923d2d0)
...
commit 9f500d6cf5eaa49391d6deca85fc864e5bd23415 (8923d2d0^)
...
commit f79c24a291a58845b08cfec7573e22cc153693e1 (8923d2d0~2)
...
commit c921c5bb63baaa16dc760de9549da55c8c89dc9c (upstream/pu)
...
commit 16793ba6b6333ba0cdee1adb53d979c3fbdb17bc (upstream/pu^)
...

Granted, this is still a little more cumbersome to type than @h1, but
then, you can skip those round-robin games as well as the possibly
backwards-incompatible extension of the rev syntax.

Ciao,
Johannes

Re: [PATCH v3 14/21] read-cache: touch shared index files when used

[Duy Nguyen]

On Fri, Jan 20, 2017 at 2:00 AM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano  wrote:
>>> Duy Nguyen  writes:
>>>
 On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> So what should we do if freshen_file() returns 0 which means that the
>> freshening failed?
>
> You tell me ;-)  as you are the one who is proposing this feature.

 My answer is, we are not worse than freshening loose objects case
 (especially since I took the idea from there).
>>>
>>> I do not think so, unfortunately.  Loose object files with stale
>>> timestamps are not removed as long as objects are still reachable.
>>
>> But there are plenty of unreachable loose objects, added in index,
>> then got replaced with new versions. cache-tree can create loose trees
>> too and it's been run more often, behind user's back, to take
>> advantage of the shortcut in unpack-trees.
>
> I am not sure if I follow.  Aren't objects reachable from the
> cache-tree in the index protected from gc?

I think the problem is loose objects created then gc run just before
they are referenced (e.g. index written down). But I think I may be
following a wrong road. If mtime is in fact to deal with race
conditions, applying the same idea here is wrong because we have a
different problem here.

> Not that I think freshening would actually fail in a repository
> where you can actually write into to update the index or its refs to
> make a difference (iow, even if we make it die() loudly when shared
> index cannot be "touched" because we are paranoid, no real life
> usage will trigger that die(), and if a repository does trigger the
> die(), I think you would really want to know about it).
-- 
Duy

[PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard

[Nguyễn Thái Ngọc Duy]

OK This patch is horrible. Though the idea is cool and I've found it
very useful. So here it is. Perhaps the idea may be revised a bit
that's more suitable for more than one user.

The problem is old, SHA-1 name is not keyboard-friendly, even in
abbreviated form. And recent change has made abbrev form longer,
harder to type. Most of the time I just go with copy/paste with the
mouse, which I don't like. name-rev helps a bit, but it's still long
to type (especially with all the ^ and ~ that requires holding shift
down).

So, this patch adds an option --mark-commits to git-log. Whenever a
commit SHA-1 is printed on screen, it gets a new shortcut in form of
 e.g. @h2. The number keeps increasing
during one git-log, so you'll get @h1, @h2, @h3 and so on. Typing @h3
gets you back to the associated SHA-1.

Different git-log runs will change the  part. So the
second git-log run may give you @j1, @j2, @j3... This is useful
because sometimes you want to run more than one command, then
reference back to commits from the first command.

There's a limited number of letters to be used here. I limit it to the
letters on the home row to avoid moving fingers too much. Which means
after like 9 runs, @h is recycled for something new. Not great if you
have "git blah blah @h2" in bash history and keep recalling it from
time to time.

So, whenever you use a shortcut, the shortcut is kept in a "database"
and will not be recycled for quite some time. When @h is recycled,
you'll get @h1, then @h3 and so on if @h2 has just been used recently.
This makes sure recently used shortcuts live on.

That's it. I have to say typing 3-4 characters is really nice. I kinda
want to reduce the number of keystokes further down, but it's been good
enough that I'm not so pissed to spend time tuning it further.

This shortcut format violates the extended sha1 syntax, I think. In
order have any chance of entering git, we'll have to deal with that
first.

---
 Makefile|   1 +
 commit.h|   1 +
 log-tree.c  |   7 +++
 pretty.c|   5 ++
 rev-counter.c (new) | 166 
 rev-counter.h (new) |  10 
 revision.c  |   4 ++
 revision.h  |   1 +
 sha1_name.c |   8 +++
 9 files changed, 203 insertions(+)
 create mode 100644 rev-counter.c
 create mode 100644 rev-counter.h

diff --git a/Makefile b/Makefile
index de5a030256..28948bd9eb 100644
--- a/Makefile
+++ b/Makefile
@@ -788,6 +788,7 @@ LIB_OBJS += replace_object.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
 LIB_OBJS += revision.o
+LIB_OBJS += rev-counter.o
 LIB_OBJS += run-command.o
 LIB_OBJS += send-pack.o
 LIB_OBJS += sequencer.o
diff --git a/commit.h b/commit.h
index b06db4d5d9..8f67687208 100644
--- a/commit.h
+++ b/commit.h
@@ -155,6 +155,7 @@ struct pretty_print_context {
struct string_list *mailmap;
int color;
struct ident_split *from_ident;
+   int mark_commits;
 
/*
 * Fields below here are manipulated internally by pp_* functions and
diff --git a/log-tree.c b/log-tree.c
index 78a5381d0e..f49366b376 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -11,6 +11,7 @@
 #include "gpg-interface.h"
 #include "sequencer.h"
 #include "line-log.h"
+#include "rev-counter.h"
 
 static struct decoration name_decoration = { "object names" };
 static int decoration_loaded;
@@ -593,6 +594,9 @@ void show_log(struct rev_info *opt)
}
opt->shown_one = 1;
 
+   if (opt->mark_commits)
+   mark_commit(>object.oid);
+
/*
 * If the history graph was requested,
 * print the graph, up to this commit's line
@@ -615,6 +619,8 @@ void show_log(struct rev_info *opt)
put_revision_mark(opt, commit);
fputs(find_unique_abbrev(commit->object.oid.hash, 
abbrev_commit),
  stdout);
+   if (opt->mark_commits)
+   printf(" @%s", oid_to_commit_mark(>object.oid));
if (opt->print_parents)
show_parents(commit, abbrev_commit);
if (opt->children.name)
@@ -687,6 +693,7 @@ void show_log(struct rev_info *opt)
ctx.output_encoding = get_log_output_encoding();
if (opt->from_ident.mail_begin && opt->from_ident.name_begin)
ctx.from_ident = >from_ident;
+   ctx.mark_commits = opt->mark_commits;
pretty_print_commit(, commit, );
 
if (opt->add_signoff)
diff --git a/pretty.c b/pretty.c
index c3ec430220..14e0022085 100644
--- a/pretty.c
+++ b/pretty.c
@@ -10,6 +10,7 @@
 #include "color.h"
 #include "reflog-walk.h"
 #include "gpg-interface.h"
+#include "rev-counter.h"
 
 static char *user_format;
 static struct cmt_fmt_map {
@@ -1127,6 +1128,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
case 'H':   /* commit hash */
strbuf_addstr(sb, diff_get_color(c->auto_color, DIFF_COMMIT));