Re: Which branch(es) contain certain commits? (was Re: (unknown))

2015-08-16 Thread Ivan Chernyavsky


15.08.2015, 12:19, "Duy Nguyen" :
>
> Probably because nobody is interested and steps up to do it. The lack
> of response to you mail is a sign. Maybe you can try make a patch? I
> imagine it would not be so different from current --contains code, but
> this time we need to look into commits, not just commit id.
>

Yeah thanks much, I'll try. Actually that was my original intention, I just 
wanted to have some "reasonability check" for my idea.

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


Re: [PATCH v11 03/13] ref-filter: introduce ref_formatting_state

2015-08-16 Thread Eric Sunshine
On Sun, Aug 16, 2015 at 7:31 PM, Eric Sunshine  wrote:
> On Sat, Aug 15, 2015 at 2:00 PM, Karthik Nayak  wrote:
>> +struct ref_formatting_state {
>> +   struct strbuf output;
>> +   struct ref_formatting_state *prev;
>
> Upon initial read-through of this patch, I found the name 'prev'
> confusing since it seems you sometimes treat this as a linked-list
> and, for a linked-list, this member is customarily named 'next'.
> However, you also sometimes treat it as a stack, in which case 'prev'
> makes a certain amount of sense semantically, although so does 'next'.
> I'd probably have named it 'next', however, attr.c:struct attr_stack
> names its member 'prev', so there is some precedence for the current
> choice.

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


Re: [PATCH v11 06/13] ref-filter: add option to filter out tags, branches and remotes

2015-08-16 Thread Eric Sunshine
On Mon, Aug 17, 2015 at 12:42 AM, Eric Sunshine  wrote:
> On Sat, Aug 15, 2015 at 2:00 PM, Karthik Nayak  wrote:
>> -   if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
>> -   ret = for_each_rawref(ref_filter_handler, &ref_cbdata);
>> -   else if (type & FILTER_REFS_ALL)
>> -   ret = for_each_ref(ref_filter_handler, &ref_cbdata);
>> -   else if (type)
>> +   if (type & FILTER_REFS_INCLUDE_BROKEN) {
>> +   type -= FILTER_REFS_INCLUDE_BROKEN;
>
> The above is a somewhat unusual way to say the more idiomatic:
>
> type &= ~FILTER_REFS_INCLUDE_BROKEN;
>
> when dealing with bit flags. Is there precedence elsewhere in the
> project for choosing '-' over '~'?

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


Re: [PATCH v11 13/13] tag.c: implement '--merged' and '--no-merged' options

2015-08-16 Thread Eric Sunshine
On Sat, Aug 15, 2015 at 2:00 PM, Karthik Nayak  wrote:
> Using 'ref-filter' APIs implement the '--merged' and '--no-merged'

s/implement/to implement/

> options into 'tag.c'. The '--merged' option lets the user to only
> list tags merged into the named commit. The '--no-merged' option
> lets the user to only list tags not merged into the named commit.
> If no object is provided it assumes HEAD as the object.
>
> Add documentation and tests for the same.
>
> Signed-off-by: Karthik Nayak 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v11 08/13] ref-filter: add support to sort by version

2015-08-16 Thread Eric Sunshine
On Sat, Aug 15, 2015 at 2:00 PM, Karthik Nayak  wrote:
> Add support to sort by version using the "v:refname" and
> "version:refname" option. This is achieved by using the 'versioncmp()'
> function as the comparing function for qsort.
>
> This option is included to support sorting by versions in `git tag -l`
> which will eventaully be ported to use ref-filter APIs.

s/eventaully/eventually/

> Add documentation and tests for the same.
>
> Signed-off-by: Karthik Nayak 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v11 06/13] ref-filter: add option to filter out tags, branches and remotes

2015-08-16 Thread Eric Sunshine
On Sat, Aug 15, 2015 at 2:00 PM, Karthik Nayak  wrote:
> Add a function called 'for_each_reftype_fullpath()' to refs.{c,h}
> which iterates through each ref for the given path without trimming
> the path and also accounting for broken refs, if mentioned.
>
> Add 'filter_ref_kind()' in ref-filter.c to check the kind of ref being
> handled and return the kind to 'ref_filter_handler()', where we
> discard refs which we do not need and assign the kind to needed refs.
>
> Signed-off-by: Karthik Nayak 
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> index eac99d0..abcd235 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1226,16 +1262,29 @@ int filter_refs(struct ref_array *array, struct 
> ref_filter *filter, unsigned int
>  {
> struct ref_filter_cbdata ref_cbdata;
> int ret = 0;
> +   unsigned int broken = 0;
>
> ref_cbdata.array = array;
> ref_cbdata.filter = filter;
>
> /*  Simple per-ref filtering */
> -   if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
> -   ret = for_each_rawref(ref_filter_handler, &ref_cbdata);
> -   else if (type & FILTER_REFS_ALL)
> -   ret = for_each_ref(ref_filter_handler, &ref_cbdata);
> -   else if (type)
> +   if (type & FILTER_REFS_INCLUDE_BROKEN) {
> +   type -= FILTER_REFS_INCLUDE_BROKEN;

The above is a somewhat unusual way to say the more idiomatic:

type &= ~FILTER_REFS_INCLUDE_BROKEN;

when dealing with bit flags. Is there precedence elsewhere in the
project for choosing '-' over '~'?

> +   broken = 1;
> +   }
> +
> +   filter->kind = type;
> +   if (type == FILTER_REFS_BRANCHES)
> +   ret = for_each_reftype_fullpath(ref_filter_handler, 
> "refs/heads/", broken, &ref_cbdata);
> +   else if (type == FILTER_REFS_REMOTES)
> +   ret = for_each_reftype_fullpath(ref_filter_handler, 
> "refs/remotes/", broken, &ref_cbdata);
> +   else if (type == FILTER_REFS_TAGS)
> +   ret = for_each_reftype_fullpath(ref_filter_handler, 
> "refs/tags/", broken, &ref_cbdata);
> +   else if (type & FILTER_REFS_ALL) {
> +   ret = for_each_reftype_fullpath(ref_filter_handler, "", 
> broken, &ref_cbdata);

These cases are all the same except for the (string) second argument,
aren't they? This might be less noisy and easier to follow if you
assign the appropriate string to a variable first, and then invoke
for_each_reftype_fullpath() once with the string variable as an
argument.

> +   if (type & FILTER_REFS_DETACHED_HEAD)
> +   head_ref(ref_filter_handler, &ref_cbdata);
> +   } else
> die("filter_refs: invalid type");
>
> /*  Filters that need revision walking */
> diff --git a/ref-filter.h b/ref-filter.h
> index 144a633..64fedd3 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -14,8 +14,14 @@
> -#define FILTER_REFS_INCLUDE_BROKEN 0x1
> -#define FILTER_REFS_ALL 0x2
> +#define FILTER_REFS_INCLUDE_BROKEN 0x0001
> +#define FILTER_REFS_TAGS   0x0002
> +#define FILTER_REFS_BRANCHES   0x0004
> +#define FILTER_REFS_REMOTES0x0008
> +#define FILTER_REFS_OTHERS 0x0010
> +#define FILTER_REFS_ALL(FILTER_REFS_TAGS | FILTER_REFS_BRANCHES 
> | \
> +   FILTER_REFS_REMOTES | FILTER_REFS_OTHERS)
> +#define FILTER_REFS_DETACHED_HEAD  0x0020

I suppose there's some good reason that FILTER_REFS_DETACHED_HEAD is
not a member of FILTER_REFS_ALL? Perhaps add a comment explaining it?

> diff --git a/refs.c b/refs.c
> index 2db2975..0f18c34 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2145,6 +2145,13 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)
>strlen(git_replace_ref_base), 0, cb_data);
>  }
>
> +int for_each_reftype_fullpath(each_ref_fn fn, char *type, unsigned int 
> broken, void *cb_data)
> +{
> +   if (broken)
> +   broken = DO_FOR_EACH_INCLUDE_BROKEN;

It's a bit ugly and confusing to have the same variable, 'broken', act
as both a boolean input argument and as a bit flag argument to the
called function.

> +   return do_for_each_ref(&ref_cache, type, fn, 0, broken, cb_data);
> +}
> +
>  int head_ref_namespaced(each_ref_fn fn, void *cb_data)
>  {
> struct strbuf buf = STRBUF_INIT;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v11 05/13] ref-filter: implement an `align` atom

2015-08-16 Thread Eric Sunshine
On Sat, Aug 15, 2015 at 2:00 PM, Karthik Nayak  wrote:
> Implement an `align` atom which left-, middle-, or right-aligns the
> content between %(align:..) and %(end).
>
> It is followed by `:,`, where the `` is
> either left, right or middle and `` is the size of the area
> into which the content will be placed. If the content between
> %(align:) and %(end) is more than the width then no alignment is
> performed. e.g. to align a refname atom to the middle with a total
> width of 40 we can do: --format="%(align:middle,40)%(refname)%(end)".
>
> This is done by calling the strbuf_utf8_align() function in utf8.c.
>
> Add documentation and tests for the same.
>
> Signed-off-by: Karthik Nayak 
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> index 3259363..eac99d0 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -10,6 +10,7 @@
>  #include "quote.h"
>  #include "ref-filter.h"
>  #include "revision.h"
> +#include "utf8.h"
>
>  typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
>
> @@ -53,16 +54,27 @@ static struct {
> { "flag" },
> { "HEAD" },
> { "color" },
> +   { "align" },
> +   { "end" },
> +};
> +
> +struct align {
> +   align_type position;
> +   unsigned int width;
>  };
>
>  struct ref_formatting_state {
> struct strbuf output;
> struct ref_formatting_state *prev;
> +   void (*attend)(struct ref_formatting_state *state);

Junio's suggestion for this member was "at end"; that is what to do
when you are "at"-the-%(end), not "attend", which isn't meaningful
here. You could also call it 'end_scope', 'finish' or 'close' or
'finalize' or something.

> +   void *cb_data;
> int quote_style;
>  };
>
>  struct atom_value {
> const char *s;
> +   struct align *align;
> +   void (*handler)(struct atom_value *atomv, struct ref_formatting_state 
> **state);
> unsigned long ul; /* used for sorting when not FIELD_STR */
>  };
>
> @@ -137,12 +149,12 @@ int parse_ref_filter_atom(const char *atom, const char 
> *ep)
>
>  static struct ref_formatting_state *push_new_state(struct 
> ref_formatting_state **state)
>  {
> -   struct ref_formatting_state *new_state = xcalloc(1, sizeof(struct 
> ref_formatting_state));
> -   struct ref_formatting_state *tmp = *state;
> +   struct ref_formatting_state *new = xcalloc(1, sizeof(struct 
> ref_formatting_state));
> +   struct ref_formatting_state *old = *state;
>
> -   *state = new_state;
> -   new_state->prev = tmp;
> -   return new_state;
> +   *state = new;
> +   new->prev = old;
> +   return new;
>  }

What are these changes about? They appear only to be renaming some
variables which were introduced in patch 3. It would make more sense
to give them the desired names in the patch which introduces them.

>  static void pop_state(struct ref_formatting_state **state)
> @@ -625,6 +637,34 @@ static inline char *copy_advance(char *dst, const char 
> *src)
> return dst;
>  }
>
> +static void align_handler(struct ref_formatting_state *state)

The names 'align_handler' and 'align_atom_handler' are confusingly
similar. Perhaps name this end_align() or do_align() or
apply_alignment() or something?

> +{
> +   struct strbuf aligned = STRBUF_INIT;
> +   struct ref_formatting_state *return_to = state->prev;
> +   struct align *align = (struct align *)state->cb_data;
> +
> +   strbuf_utf8_align(&aligned, align->position, align->width, 
> state->output.buf);
> +   strbuf_addbuf(&return_to->output, &aligned);

A couple comments:

First, why is 'strbuf aligned' needed? Can't you instead just invoke
strbuf_utf8_align(&return_to->output, ...)?

Second, I realize that Junio suggested the 'return_to' idea, but it
seems like it could become overly painful since each handler of this
sort is going to have to perform the same manipulation to append its
collected output to its parent state's output. What if you instead
make it the responsibility of pop_state() to append the 'output' from
the state being popped to the "prev" state's 'output'? This way, it
happens automatically, thus reducing code in each individual handler,
and reducing the burden of having to keep writing the same code.

> +   strbuf_release(&aligned);
> +}
> +
> +static void align_atom_handler(struct atom_value *atomv, struct 
> ref_formatting_state **state)
> +{
> +   struct ref_formatting_state *new = push_new_state(state);
> +   strbuf_init(&new->output, 0);

I think this strbuf_init() should be the responsibility of
push_new_state(), as mentioned in my patch 3 review, otherwise every
caller of push_new_state() will have to remember to do this.

> +   new->attend = align_handler;
> +   new->cb_data = atomv->align;
> +}
> +
> +static void end_atom_handler(struct atom_value *atomv, struct 
> ref_formatting_state **state)
> +{
> +   struct ref_formatting_state *current = *state;
> +   if (!current->attend)
> +   die(_("form

Re: [PATCH v11 05/13] ref-filter: implement an `align` atom

2015-08-16 Thread Eric Sunshine
On Sun, Aug 16, 2015 at 8:04 AM, Andreas Schwab  wrote:
> Karthik Nayak  writes:
>> I think we need to squash this in
>>
>> diff --git a/Documentation/git-for-each-ref.txt
>> b/Documentation/git-for-each-ref.txt
>> index 3099631..17bd15e 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -129,7 +129,7 @@ color::
>>
>>  align::
>> left-, middle-, or right-align the content between %(align:..)
>> -   and %(end). Followed by `:,`, where the
>> +   and %(end). Followed by `:>,`, where the
>
> s/>>/>/

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


Re: [PATCH v11 04/13] utf8: add function to align a string into given strbuf

2015-08-16 Thread Eric Sunshine
On Sat, Aug 15, 2015 at 2:00 PM, Karthik Nayak  wrote:
> Add strbuf_utf8_align() which will align a given string into a strbuf
> as per given align_type and width. If the width is greater than the
> string length then no alignment is performed.

A couple minor comments below...

> Signed-off-by: Karthik Nayak 
> ---
> diff --git a/utf8.c b/utf8.c
> index 28e6d76..0fb8e9d 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -644,3 +644,24 @@ int skip_utf8_bom(char **text, size_t len)
> *text += strlen(utf8_bom);
> return 1;
>  }
> +
> +void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int 
> width,
> +  const char *s)
> +{
> +   int slen = strlen(s);
> +   int display_len = utf8_strnwidth(s, slen, 0);
> +   int utf8_compensation = slen - display_len;

Based upon the previous round review, I think you had intended to name
this merely 'compensation'.

> +   if (display_len >= width) {
> +   strbuf_addstr(buf, s);
> +   return;
> +   }
> +
> +   if (position == ALIGN_LEFT)
> +   strbuf_addf(buf, "%-*s", width + utf8_compensation, s);
> +   else if (position == ALIGN_MIDDLE) {
> +   int left = (width - display_len)/2;

Style: spaces around '/'

> +   strbuf_addf(buf, "%*s%-*s", left, "", width - left + 
> utf8_compensation, s);
> +   } else if (position == ALIGN_RIGHT)
> +   strbuf_addf(buf, "%*s", width + utf8_compensation, s);
> +}
> diff --git a/utf8.h b/utf8.h
> index 5a9e94b..7930b44 100644
> --- a/utf8.h
> +++ b/utf8.h
> @@ -55,4 +55,19 @@ int mbs_chrlen(const char **text, size_t *remainder_p, 
> const char *encoding);
>   */
>  int is_hfs_dotgit(const char *path);
>
> +typedef enum {
> +   ALIGN_LEFT,
> +   ALIGN_MIDDLE,
> +   ALIGN_RIGHT
> +} align_type;
> +
> +/*
> + * Align the string given and store it into a strbuf as per the
> + * 'position' and 'width'. If the given string length is larger than
> + * 'width' than then the input string is not truncated and no
> + * alignment is done.
> + */
> +void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int 
> width,
> +  const char *s);
> +
>  #endif
> --
> 2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: "git am --abort" screwing up index?

2015-08-16 Thread Linus Torvalds
On Sun, Aug 16, 2015 at 12:46 PM, Linus Torvalds
 wrote:
>
> Maybe it has always done this, and I just haven't noticed (I usually
> _just_ do the "git reset --hard" thing, don't ask me why I wanted to
> be doubly sure this time). But maybe it's an effect of the new
> built-in "am".

I bisected this. It's definitely used to work, and the regression is
from the new built-in am. But I cannot bisect into that branch
'pt/am-builtin', because "git am" doesn't actually work in the middle
of that branch.

So I've verified that commit c1e5ca90dba8 ("Merge branch
'es/worktree-add'") is good, and that commit 7aa2da616208 ("Merge
branch 'pt/am-builtin'") is bad, but I cannot pinpoint the exact
commit where "git am --abort" starts breaking the index.

But I assume it's simply that initial implementation of "--abort" in
commit 33388a71d23e ("builtin-am: implement --abort") that already
ends up rewriting the index from scratch without applying the old stat
data.

The test-case is pretty simple: just force a "git am" failure, then do
"git am --abort", and then you can check whether the index stat()
information is valid in various ways. For the kernel, doing a "git
reset --hard" makes it obvious because the reset will force all files
to be written out (since the index stat information doesn't match the
current tree). But you can do it by just counting system calls for a
"git diff" too. On the git tree, for example, when the index has
matching stat information, you get something like

  [torvalds@i7 git]$ strace -cf git diff
  ..
0.040.25   126 4 open
  ..

ie you only actually ended up with 26 open() system calls. When the
index is not in sync with the stat information, "git diff" will have
to open each file to see what the actual contents are, and you get

  [torvalds@i7 git]$ strace -cf git diff
  ...
0.300.70   0  5987   302 open
  ...

so now it opened about 6k files instead (and for the kernel, that
number will be much larger, of course).

I _think_ it's because git-am (in "clean_index()") uses read_tree(),
while it probably should use "unpack_trees" with opts.update and
opts.reset set (like reset_index() does in builtin/reset.h).

I have to go off do my weekly -rc now, and probably won't get to
debugging this much further. Adding Stefan to the cc, since he helped
with that "--abort" implementation.

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


Re: [PATCH v11 03/13] ref-filter: introduce ref_formatting_state

2015-08-16 Thread Eric Sunshine
On Sat, Aug 15, 2015 at 2:00 PM, Karthik Nayak  wrote:
> Introduce ref_formatting_state which will hold the formatted
> output strbuf and is used for nesting of modifier atoms.
>
> Signed-off-by: Karthik Nayak 
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> index edfb1c7..3259363 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -55,6 +55,12 @@ static struct {
> { "color" },
>  };
>
> +struct ref_formatting_state {
> +   struct strbuf output;
> +   struct ref_formatting_state *prev;

Upon initial read-through of this patch, I found the name 'prev'
confusing since it seems you sometimes treat this as a linked-list
and, for a linked-list, this member is customarily named 'next'.
However, you also sometimes treat it as a stack, in which case 'prev'
makes a certain amount of sense semantically, although so does 'next'.
I'd probably have named it 'next', however, attr.c:struct attr_stack
names its member 'prev', so there is some precedence for the current
choice.

Also, it's customary for this member to be the first (or less
frequently last) member in the structure.

> +   int quote_style;
> +};
> +
>  struct atom_value {
> const char *s;
> unsigned long ul; /* used for sorting when not FIELD_STR */
> @@ -129,6 +135,26 @@ int parse_ref_filter_atom(const char *atom, const char 
> *ep)
> return at;
>  }
>
> +static struct ref_formatting_state *push_new_state(struct 
> ref_formatting_state **state)

This interface seems confused. The caller receives the new state as
both the return value of the function and as an out-value of its sole
argument. I'd suggest choosing one or the other.

Which one you choose depends upon how you view the operation and the
data structure. If you view it as linked-list manipulation, then you'd
probably want the new state returned, and accept a pointer argument
(rather than pointer-to-pointer). On the other hand, if you view it as
a stack, then you'd probably want to have it return void and
manipulate the sole argument. For this case, it might be clearer to
name the argument 'stack' rather than 'state'.

> +{
> +   struct ref_formatting_state *new_state = xcalloc(1, sizeof(struct 
> ref_formatting_state));
> +   struct ref_formatting_state *tmp = *state;
> +
> +   *state = new_state;
> +   new_state->prev = tmp;
> +   return new_state;
> +}

A couple issues:

First, you need to initialize the strbuf member of
ref_formatting_state after allocation:

new_state = xcalloc(1, sizeof(struct ref_formatting_state));
strbuf_init(&new_state->output, 0);

Second, if you re-order the code slightly, the 'tmp' variable becomes
unnecessary.

Assuming that your intention was to match pop_state() and treat this
opaquely as a stack rather than exposing its linked-list
implementation, I'd have expected the function to look something like
this:

static void push_new_state(struct ref_formatting_state **stack)
{
struct ref_formatting_state *s = xcalloc(...);
s->next = *stack;
strbuf_init(&s->output, 0);
*stack = s;
}

> +static void pop_state(struct ref_formatting_state **state)
> +{
> +   struct ref_formatting_state *current = *state;
> +   struct ref_formatting_state *prev = current->prev;
> +
> +   strbuf_release(¤t->output);
> +   free(current);
> +   *state = prev;
> +}

This interface suggests that you're treating it as an opaque stack, in
which case naming the argument 'stack' might be clearer.

>  /*
>   * In a format string, find the next occurrence of %(atom).
>   */
> @@ -1195,23 +1221,23 @@ void ref_array_sort(struct ref_sorting *sorting, 
> struct ref_array *array)
> qsort(array->items, array->nr, sizeof(struct ref_array_item *), 
> compare_refs);
>  }
>
> -static void append_atom(struct atom_value *v, int quote_style, struct strbuf 
> *output)
> +static void append_atom(struct atom_value *v, struct ref_formatting_state 
> *state)
>  {
> -   switch (quote_style) {
> +   switch (state->quote_style) {
> case QUOTE_NONE:
> -   strbuf_addstr(output, v->s);
> +   strbuf_addstr(&state->output, v->s);
> break;
> case QUOTE_SHELL:
> -   sq_quote_buf(output, v->s);
> +   sq_quote_buf(&state->output, v->s);
> break;
> case QUOTE_PERL:
> -   perl_quote_buf(output, v->s);
> +   perl_quote_buf(&state->output, v->s);
> break;
> case QUOTE_PYTHON:
> -   python_quote_buf(output, v->s);
> +   python_quote_buf(&state->output, v->s);
> break;
> case QUOTE_TCL:
> -   tcl_quote_buf(output, v->s);
> +   tcl_quote_buf(&state->output, v->s);
> break;
> }
>  }

This patch touches all the same lines as the previous patch which
converted the code to append to a strbuf rather than emit to stdout,
thus it makes the previous 

"git am --abort" screwing up index?

2015-08-16 Thread Linus Torvalds
So I just noticed while applying a patch with "git am" when I had a
dirty tree, and I ended up getting a failure and starting over:

   [torvalds@i7 linux]$ git am --abort
   [torvalds@i7 linux]$ git reset --hard
   Checking out files: 100% (50794/50794), done.0794)
   HEAD is now at 1efdb5f0a924 Merge tag 'scsi-fixes' of
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi

and the thing I reacted to is that the "git reset --hard" re-checked
out all the files.

That implies that "git am --abort" ended up leaving the index in a bad
state, presumably it re-did the index entirely from HEAD, without
filling it in with the stat() details from the old index.

Maybe it has always done this, and I just haven't noticed (I usually
_just_ do the "git reset --hard" thing, don't ask me why I wanted to
be doubly sure this time). But maybe it's an effect of the new
built-in "am".

I'm about to go out and don't have time to debug this any further
right now, but I'll try to get back to it later. I thought I'd send
out this email in case it makes Paul goes "ahh, yes.. obvious"

Not a big deal - things *work* fine. But forcing checking out every
file obviously also means that subsequent builds end up being slowed
down etc.,.

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


Re: [PATCH v3] po/README: Update directions for l10n contributors

2015-08-16 Thread Philip Oakley

From: "Jiang Xin" 

From: Philip Oakley 

Some Linux distributions (such as Ubuntu) have their own l10n 
workflows,

and their translations may be different.  Add notes for this case for
l10n translators.

Signed-off-by: Philip Oakley 
Signed-off-by: Jiang Xin 
---
po/README | 19 +++
1 file changed, 19 insertions(+)

diff --git a/po/README b/po/README
index d8c9111..fef4c0f 100644
--- a/po/README
+++ b/po/README
@@ -10,10 +10,26 @@ coordinates our localization effort in the l10 
coordinator repository:


https://github.com/git-l10n/git-po/

+The two character language translation codes are defined by 
ISO_639-1, as
+stated in the gettext(1) full manual, appendix A.1, Usual Language 
Codes.

+
+
+Contributing to an existing translation
+---
As a contributor for a language XX, you should first check TEAMS file 
in
this directory to see whether a dedicated repository for your language 
XX

exists. Fork the dedicated repository and start to work if it exists.

+Sometime, contributors may find that the translations of their Git
+distributions are quite different with the translations of the
+corresponding version from Git official. This is because some Git
+distributions (such as from Ubuntu, etc.) have their own l10n 
workflow.
+For this case, wrong translations should be reported and fixed 
through

+their workflows.
+


OK. That's a reasonable summary of what the reader should do.



+
+Creating a new language translation
+---
If you are the first contributor for the language XX, please fork this
repository, prepare and/or update the translated message file po/XX.po
(described later), and ask the l10n coordinator to pull your work.
@@ -23,6 +39,9 @@ coordinate among yourselves and nominate the team 
leader for your

language, so that the l10n coordinator only needs to interact with one
person per language.

+
+Translation Process Flow
+
The overall data-flow looks like this:

+---++--+
--


Confirmed-by: Philip Oakley 
--

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


Re: [PATCH v2 0/1] handling mistranslation reports

2015-08-16 Thread Philip Oakley

From: "Jiang Xin" 

2015-08-04 6:29 GMT+08:00 Philip Oakley :

Hi Jiang,

This is my updated patch based on your feedback at $gmane/275141
and $gmane/275142.

I've used most of your wording, though have retained a comment on
considering if the translation could be held here.

My original commentary is below, along with the inter-diff between 
versions.


-
Recently, on the 'Git for human beings' list, a user reported a
mistranslation and asked if/what could be done, with a suggested
alternate text [1].

I pointed the user at the po/README for general guidance.
Unfortunately the user was noting a Spanish (es) translation error 
which
is not held in your tree, but the README does include how to start a 
new
translation. This led to a misunderstanding with regard to which 
aspect

of the README was being referred to (private discussion with Junio).

This patch separates out the three different aspects that caused 
confusion
and explicitly brings out what to do for translations not currently 
held here.


I hope my suggested patch will fit with your approach and ask for 
comments

(or Ack / Nack).

regards

Philip

[1] https://groups.google.com/forum/#!topic/git-users/rK5oU6k8Tzw

Interdiff:


The subject is "[PATCH v2 0/1]" and I wonder where is the real patch
file ("v2 1/1")
aside of this  cover letter.


Yes 0/1 was the cover letter and 1/1 was the actual patch.

I have not found a way of sending, via my 'git send-email' (*), a single 
patch with its  integrated cover-letter at the beginning. I can easily 
add short notes after a 3-dashes in the commit, but it is not the same 
as a cover-letter ...


Hopefully, you got the 1/1. Unfortunately, the Git list is rejecting my 
patches (and just my patches), as sent via send-email, and at the moment 
I have no way of debugging and diagnosing that issue.



But today I have time to read it
carefully, and I know
this is a fixup commit.

I squash this reroll to last commit for this series, and simplify both
the commit
and the commit log.  See patch v3 in the next email.


Thanks. Will review.



--
Jiang Xin
--
(*) https://groups.google.com/forum/#!topic/msysgit/U85cSXd-Uv0 


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


Re: [PATCH v3 2/4] path: optimize common dir checking

2015-08-16 Thread Duy Nguyen
On Sun, Aug 16, 2015 at 12:04 PM, David Turner  wrote:
> Duy Nguyen  writes:
>> On Thu, Aug 13, 2015 at 4:57 AM, David Turner 
> wrote:
>> > Instead of a linear search over common_list to check whether
>> > a path is common, use a trie.  The trie search operates on
>> > path prefixes, and handles excludes.
>>
>> Just be careful that the given key from git_path is not normalized. I
>> think you assume it is in the code, but I haven't read carefully. We
>> could of course optimize for the good case: assume normalized and
>> search, then fall back to explicit normalizing and search again.
>
> What does it mean for a key to be normalized?  Do you mean normalized in
> terms of upper/lowercase on case-insensitive filesystems?  If so, I think the
> assumption here is that this will be called with paths generated by git,
> which will always use the lowercase path.

Mostly about duplicated slashes, "abc//def" instead of "abc/def".
Technically nothing forbids git_path("refs/../refs/foo"), but that
would fool even current code.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] untracked-cache: fix subdirectory handling

2015-08-16 Thread Duy Nguyen
On Sun, Aug 16, 2015 at 12:17 PM, David Turner  wrote:
> Previously, some calls lookup_untracked would pass a full path.  But
> lookup_untracked assumes that the portion of the path up to and
> including to the untracked_cache_dir has been removed.  So
> lookup_untracked would be looking in the untracked_cache for 'foo' for
> 'foo/bar' (instead of just looking for 'bar').  This would cause
> untracked cache corruption.
>
> Instead, treat_directory learns to track the base length of the parent
> directory, so that only the last path component is passed to
> lookup_untracked.

Your v2 also fixes untracked_cache_invalidate_path(), which is not
included here. Maybe it's in another patch?

> Helped-by: Nguyễn Thái Ngọc Duy 
> Signed-off-by: David Turner 
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>
> This version incorporates Duy's version of the dir.c code, and his
> suggested test speedup.
>
> ---
>  dir.c | 14 
>  t/t7063-status-untracked-cache.sh | 74 
> +--
>  2 files changed, 80 insertions(+), 8 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index e7b89fe..cd4ac77 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1297,7 +1297,7 @@ static enum exist_status 
> directory_exists_in_index(const char *dirname, int len)
>   */
>  static enum path_treatment treat_directory(struct dir_struct *dir,
> struct untracked_cache_dir *untracked,
> -   const char *dirname, int len, int exclude,
> +   const char *dirname, int len, int baselen, int exclude,
> const struct path_simplify *simplify)
>  {
> /* The "len-1" is to strip the final '/' */
> @@ -1324,7 +1324,8 @@ static enum path_treatment treat_directory(struct 
> dir_struct *dir,
> if (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))
> return exclude ? path_excluded : path_untracked;
>
> -   untracked = lookup_untracked(dir->untracked, untracked, dirname, len);
> +   untracked = lookup_untracked(dir->untracked, untracked,
> +dirname + baselen, len - baselen);
> return read_directory_recursive(dir, dirname, len,
> untracked, 1, simplify);
>  }
> @@ -1444,6 +1445,7 @@ static int get_dtype(struct dirent *de, const char 
> *path, int len)
>  static enum path_treatment treat_one_path(struct dir_struct *dir,
>   struct untracked_cache_dir 
> *untracked,
>   struct strbuf *path,
> + int baselen,
>   const struct path_simplify 
> *simplify,
>   int dtype, struct dirent *de)
>  {
> @@ -1495,8 +1497,8 @@ static enum path_treatment treat_one_path(struct 
> dir_struct *dir,
> return path_none;
> case DT_DIR:
> strbuf_addch(path, '/');
> -   return treat_directory(dir, untracked, path->buf, path->len, 
> exclude,
> -   simplify);
> +   return treat_directory(dir, untracked, path->buf, path->len,
> +  baselen, exclude, simplify);
> case DT_REG:
> case DT_LNK:
> return exclude ? path_excluded : path_untracked;
> @@ -1557,7 +1559,7 @@ static enum path_treatment treat_path(struct dir_struct 
> *dir,
> return path_none;
>
> dtype = DTYPE(de);
> -   return treat_one_path(dir, untracked, path, simplify, dtype, de);
> +   return treat_one_path(dir, untracked, path, baselen, simplify, dtype, 
> de);
>  }
>
>  static void add_untracked(struct untracked_cache_dir *dir, const char *name)
> @@ -1827,7 +1829,7 @@ static int treat_leading_path(struct dir_struct *dir,
> break;
> if (simplify_away(sb.buf, sb.len, simplify))
> break;
> -   if (treat_one_path(dir, NULL, &sb, simplify,
> +   if (treat_one_path(dir, NULL, &sb, baselen, simplify,
>DT_DIR, NULL) == path_none)
> break; /* do not recurse into it */
> if (len <= baselen) {
> diff --git a/t/t7063-status-untracked-cache.sh 
> b/t/t7063-status-untracked-cache.sh
> index ff23f4e..6716f69 100755
> --- a/t/t7063-status-untracked-cache.sh
> +++ b/t/t7063-status-untracked-cache.sh
> @@ -402,13 +402,14 @@ test_expect_success 'set up sparse checkout' '
> echo "done/[a-z]*" >.git/info/sparse-checkout &&
> test_config core.sparsecheckout true &&
> git checkout master &&
> -   git update-index --untracked-cache &&
> +   git update-index --untracked-cache --force-untracked-cache &&
> git status --porcelain >/dev/null && # prime the cache
> test_path_is_missing done/.gitignore &&
> test_path_is_file done/one
>  '
>
> -test_expect_success 'create files, some

Re: [PATCH v11 05/13] ref-filter: implement an `align` atom

2015-08-16 Thread Andreas Schwab
Karthik Nayak  writes:

> I think we need to squash this in
>
> diff --git a/Documentation/git-for-each-ref.txt
> b/Documentation/git-for-each-ref.txt
> index 3099631..17bd15e 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -129,7 +129,7 @@ color::
>
>  align::
> left-, middle-, or right-align the content between %(align:..)
> -   and %(end). Followed by `:,`, where the
> +   and %(end). Followed by `:>,`, where the

s/>>/>/

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v11 05/13] ref-filter: implement an `align` atom

2015-08-16 Thread Karthik Nayak
I think we need to squash this in

diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 3099631..17bd15e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -129,7 +129,7 @@ color::

 align::
left-, middle-, or right-align the content between %(align:..)
-   and %(end). Followed by `:,`, where the
+   and %(end). Followed by `:>,`, where the
`` is either left, right or middle and `` is
the total length of the content with alignment. If the
contents length is more than the width then no alignment is
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v11 00/13] port tag.c to use ref-filter APIs

2015-08-16 Thread Karthik Nayak
On Sun, Aug 16, 2015 at 1:08 PM, Jacob Keller  wrote:
> On Sat, Aug 15, 2015 at 11:00 AM, Karthik Nayak  wrote:
>
>>
>>  align::
>> -   Implement an `align` atom which left-, middle-, or
>> -   right-aligns the content between %(align:..)  and
>> -   %(end). Followed by `:,`, where the
>> +   left-, middle-, or right-align the content between %(align:..)
>> +   and %(end). Followed by `:,`, where the
>
>
> Everywhere else in the code seems to now put position second now, but
> the documentation here doesn't say this is allowed or required.
>

Oops! you're right, that needs to be swapped.

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


[PATCH v3] po/README: Update directions for l10n contributors

2015-08-16 Thread Jiang Xin
From: Philip Oakley 

Some Linux distributions (such as Ubuntu) have their own l10n workflows,
and their translations may be different.  Add notes for this case for
l10n translators.

Signed-off-by: Philip Oakley 
Signed-off-by: Jiang Xin 
---
 po/README | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/po/README b/po/README
index d8c9111..fef4c0f 100644
--- a/po/README
+++ b/po/README
@@ -10,10 +10,26 @@ coordinates our localization effort in the l10 coordinator 
repository:
 
 https://github.com/git-l10n/git-po/
 
+The two character language translation codes are defined by ISO_639-1, as
+stated in the gettext(1) full manual, appendix A.1, Usual Language Codes.
+
+
+Contributing to an existing translation
+---
 As a contributor for a language XX, you should first check TEAMS file in
 this directory to see whether a dedicated repository for your language XX
 exists. Fork the dedicated repository and start to work if it exists.
 
+Sometime, contributors may find that the translations of their Git
+distributions are quite different with the translations of the
+corresponding version from Git official. This is because some Git
+distributions (such as from Ubuntu, etc.) have their own l10n workflow.
+For this case, wrong translations should be reported and fixed through
+their workflows.
+
+
+Creating a new language translation
+---
 If you are the first contributor for the language XX, please fork this
 repository, prepare and/or update the translated message file po/XX.po
 (described later), and ask the l10n coordinator to pull your work.
@@ -23,6 +39,9 @@ coordinate among yourselves and nominate the team leader for 
your
 language, so that the l10n coordinator only needs to interact with one
 person per language.
 
+
+Translation Process Flow
+
 The overall data-flow looks like this:
 
 +---++--+
-- 
2.5.0.rc2.34.gfbdeabf.dirty

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


Re: [PATCH v11 00/13] port tag.c to use ref-filter APIs

2015-08-16 Thread Jacob Keller
On Sat, Aug 15, 2015 at 11:00 AM, Karthik Nayak  wrote:

>
>  align::
> -   Implement an `align` atom which left-, middle-, or
> -   right-aligns the content between %(align:..)  and
> -   %(end). Followed by `:,`, where the
> +   left-, middle-, or right-align the content between %(align:..)
> +   and %(end). Followed by `:,`, where the


Everywhere else in the code seems to now put position second now, but
the documentation here doesn't say this is allowed or required.

Regards,
Jake

> `` is either left, right or middle and `` is
> the total length of the content with alignment. If the
> contents length is more than the width then no alignment is
> -   performed. Currently nested alignment is not supported.
> +   performed.
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: feature request: better support for typos

2015-08-16 Thread Lawrence Siebert
I believe They mean that if you type "clone --branch mister", it
should ask if you meant to "clone --branch master" instead, or
something.

Basically if you type a non existent branch name, calculate edit
distance for each branch name, probably either using a timeout or edit
distance to stop if something is too different from any branch so you
don't run for too long, and then, if a branch is within a certain edit
distance from the typo, suggest it.

Lawrence

On Sat, Aug 15, 2015 at 2:12 AM, Duy Nguyen  wrote:
> On Sat, Aug 8, 2015 at 1:12 AM, Ralf Thielow  wrote:
>> Hi,
>>
>> when a user made a typo, Git is not good in guessing what
>> the user could have meant, except for git commands. I think
>> this is an area with room for improvements.
>> Let's look into branches. When I "clone --branch" and make
>> a typo, Git could show me what branch I could have meant. It's
>> the same when I try to merge or track a branch.
>
> Good candidate for those micro-projects next year.
>
>> It might even
>> be possible to show suggestions for options for all Git commands.
>
> You mean if you type "--brnch" it should suggest "--branch"? I was
> bugged about this and wanted to do something, only to realize in most
> cases git would show "git  -h", which does a much better job
> because it would explain what --branch is for as well.
>
>> What I'm trying to say is, there are arguments with a limited
>> amount of possible values that Git know, so Git can show
>> suggestions when the user made a typo for such an argument.
> --
> Duy
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
About Me: http://about.me/lawrencesiebert
Constantly Coding: http://constantcoding.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html