* Re: [PATCH 5/5] log: add --commit-header option
From: Junio C Hamano @ 2016-09-29 17:49 UTC (permalink / raw)
To: Jeff King; +Cc: Kyle J. McKay, Git mailing list
In-Reply-To: <20160929083851.kx6itvrh4n2rttrx@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> This lets you stick a header right before a commit, but
> suppresses headers that are duplicates. This means you can
> do something like:
>
> git log --graph --author-date-order --commit-header='== %as =='
>
> to get a marker in the graph whenever the day changes.
That's interesting. So it is not really "commit" header, but a
header for groups of commits. Credits for realizing the usefulness
of such grouping may go to Kyle, but the implementation is also
brilliant ;-).
> This probably needs some refactoring around the setup of the
> pretty-print context.
^ permalink raw reply
* Re: [PATCH 1/4] config: allow customizing /etc/gitconfig location
From: Matthieu Moy @ 2016-09-29 17:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narębski, git, Jeff King, Linus Torvalds
In-Reply-To: <xmqq7f9uhbc4.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Jakub Narębski <jnareb@gmail.com> writes:
>
>> W dniu 29.09.2016 o 01:30, Junio C Hamano pisze:
>>> With a new environment variable GIT_ETC_GITCONFIG, the users can
>>> specify a file that is used instead of /etc/gitconfig to read (and
>>> write) the system-wide configuration.
>>
>> Why it is named GIT_ETC_GITCONFIG (which is Unix-ism), and not
>> GIT_CONFIG_SYSTEM / GIT_CONFIG_SYSTEM_PATH, that is something
>> OS-neutral?
>
> Isn't "environment variable" something that came from POSIX world?
I don't know who invented the concept, but environment variables have
been there in the windows world since it exists I think (it existed in
MS-DOS).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH/RFC] git log --oneline alternative with dates, times and initials
From: Junio C Hamano @ 2016-09-29 17:38 UTC (permalink / raw)
To: Jeff King; +Cc: Kyle J. McKay, Git mailing list
In-Reply-To: <20160929125238.hifkxe7cmyebg64u@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> Those patches are missing some of the features like showing root commits,
>> handling two letter initials, showing the weekday, inserting a break where
>> needed to avoid parent-child confusion in graph output and properly handling
>> Duy's initials. :)
>
> I'm not too surprised. I literally looked at the first screenshot from
> your output and thought "surely git can do that with some minor tweaks".
> Nor am I surprised that there are cases where the output is funny (99%
> of the time I spent on it was tracking down that graph-padding bug).
>
> I have no problem taking this in contrib or whatever, until a point when
> Git is capable of doing the same thing itself. I just hoped to trick you
> into working on Git. :)
I thought we stopped adding random things to contrib/, though.
Unlike the earlier days of Git, if a custom command that uses Git is
very userful, it can live its own life and flourish within the much
larger Git userbase we have these days.
^ permalink raw reply
* Re: [PATCH 2/5] pretty: allow formatting names as initials
From: Jeff King @ 2016-09-29 17:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kyle J. McKay, Git mailing list
In-Reply-To: <xmqq37kihatp.fsf@gitster.mtv.corp.google.com>
On Thu, Sep 29, 2016 at 10:31:30AM -0700, Junio C Hamano wrote:
> > When I first tested it with "git log --format=%aS" I had to wonder "who
> > the heck is ntnd?". So using only the first-and-last would match the git
> > project's practice better, at least.
>
> And there is also "isalpha() good enough?" question.
>
> I think we have a few Chinese and Hangul as well as Cyrillic names
> in our history, some of them having outside-ascii first letters.
> One of the more prolific contributor's initial is ÆAB ;-)
Heh, true. In case it was not clear, these were mostly quick-and-dirty
patches. I think the right test is probably '!isspace()".
-Peff
^ permalink raw reply
* Re: [PATCH 2/5] pretty: allow formatting names as initials
From: Junio C Hamano @ 2016-09-29 17:31 UTC (permalink / raw)
To: Jeff King; +Cc: Kyle J. McKay, Git mailing list
In-Reply-To: <20160929083654.nofgkn6kwb7bavzk@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Initials are shorter and often unique enough in a
> per-project setting, so they can be used to give a more
> informative version of --oneline.
>
> The 'S' in the placeholder is for "short" (and 's' is
> already taken by DATE_SHORT), but obviously that's pretty
> arcane.
>
> Possibly there should be more customization of initials,
> asking for only 2-letter initials, etc.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> When I first tested it with "git log --format=%aS" I had to wonder "who
> the heck is ntnd?". So using only the first-and-last would match the git
> project's practice better, at least.
And there is also "isalpha() good enough?" question.
I think we have a few Chinese and Hangul as well as Cyrillic names
in our history, some of them having outside-ascii first letters.
One of the more prolific contributor's initial is ÆAB ;-)
> pretty.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/pretty.c b/pretty.c
> index c532c17..de62405 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -674,6 +674,23 @@ static int mailmap_name(const char **email, size_t *email_len,
> return mail_map->nr && map_user(mail_map, email, email_len, name, name_len);
> }
>
> +static void format_initials(struct strbuf *out, const char *name, size_t len)
> +{
> + int initial = 1;
> + size_t i;
> +
> + for (i = 0; i < len; i++) {
> + char c = name[i];
> + if (isspace(c)) {
> + initial = 1;
> + continue;
> + }
> + if (initial && isalpha(c))
> + strbuf_addch(out, tolower(c));
> + initial = 0;
> + }
> +}
> +
> static size_t format_person_part(struct strbuf *sb, char part,
> const char *msg, int len,
> const struct date_mode *dmode)
> @@ -702,6 +719,10 @@ static size_t format_person_part(struct strbuf *sb, char part,
> strbuf_add(sb, mail, maillen);
> return placeholder_len;
> }
> + if (part == 'S') {
> + format_initials(sb, name, namelen);
> + return placeholder_len;
> + }
>
> if (!s.date_begin)
> goto skip;
^ permalink raw reply
* Re: [PATCH 1/4] config: allow customizing /etc/gitconfig location
From: Junio C Hamano @ 2016-09-29 17:20 UTC (permalink / raw)
To: Jakub Narębski; +Cc: git, Jeff King, Linus Torvalds
In-Reply-To: <f239b2eb-d122-9c4b-187b-fbd40a94bcf4@gmail.com>
Jakub Narębski <jnareb@gmail.com> writes:
> W dniu 29.09.2016 o 01:30, Junio C Hamano pisze:
>> With a new environment variable GIT_ETC_GITCONFIG, the users can
>> specify a file that is used instead of /etc/gitconfig to read (and
>> write) the system-wide configuration.
>
> Why it is named GIT_ETC_GITCONFIG (which is Unix-ism), and not
> GIT_CONFIG_SYSTEM / GIT_CONFIG_SYSTEM_PATH, that is something
> OS-neutral?
Isn't "environment variable" something that came from POSIX world?
^ permalink raw reply
* Re: [PATCH 10/10] get_short_sha1: list ambiguous objects on error
From: Junio C Hamano @ 2016-09-29 17:19 UTC (permalink / raw)
To: Jeff King; +Cc: Kyle J. McKay, Linus Torvalds, Git Mailing List
In-Reply-To: <20160929130322.562ng4t2ktk6qzok@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> $ git rev-parse --disambiguate-list=b2e1
>> b2e1196 tag v2.8.0-rc1
>> b2e11d1 tree
>> b2e1632 commit 2007-11-14 - Merge branch 'bs/maint-commit-options'
>> b2e1759 blob
>> b2e18954 blob
>> b2e1895c blob
>
> I think the "right" way to do this is pipe the list of sha1s into
> another git commit which can format them however you want.
> Unfortunately, there isn't a single command that does a great job:
>
> - "cat-file --batch-check" can show you the sha1 and type, but it
> won't abbreviate sha1s, and it won't show you commit/tag information
>
> - "log --stdin --no-walk" will format the commit however you like, but
> skips the trees and blobs entirely, and the tag can only be seen via
> "%d"
>
> - "for-each-ref" has flexible formatting, too, but wants to format
> refs, not objects (and doesn't read from stdin).
- "name-rev" is used to give "describe --contains", and can read
from its standard input, but has no format customization.
Another downside of it is that it only wants to see
committishes.
> IMHO that is a sign that our formatting tools aren't as good as they
> could be (I think the right tool is cat-file, but it should be able to
> do all of the formatting that the other commands can do).
>
> Of course if you really just want human-readable output, then:
>
> $ git cat-file -e b2e1
> error: short SHA1 b2e1 is ambiguous
> hint: The candidates are:
> hint: b2e1196 tag v2.8.0-rc1
> hint: b2e11d1 tree
> hint: b2e1632 commit 2007-11-14 - Merge branch 'bs/maint-commit-options'
> hint: b2e1759 blob
> hint: b2e18954 blob
> hint: b2e1895c blob
> fatal: Not a valid object name b2e1
>
> is pretty easy.
Yes. I think adding this to rev-parse that is meant for machines is
probably a mistake, as this "hint" machinery's output will become
even more human friendly over time as we gain experience.
- If the hypothetical "--disambiguate-list" option wants to produce
machine parseable output for scripts, it would mean its output
(and whatgver the reading script can do based on its output for
humans) will become less useful for humans over time.
- If the hypothetical "--disambiguate-list" option only wants to
replicate the human readable output that is designed to be
improved over time and expects its output _not_ to be interpreted
by scripts but merely be relayed, then why aren't these scripts
just invoking the commands that already gives the "hint:" output
and showing that directly to humans in the first place?
> That being said, I don't mind if somebody wanted to do a rev-parse
> option on top of my series. The formatting code is already split into
> its own function.
So let's not go there.
^ permalink raw reply
* Re: [PATCH v2 02/11] i18n: add--interactive: mark simple here documents for translation
From: Junio C Hamano @ 2016-09-29 17:05 UTC (permalink / raw)
To: Vasco Almeida
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
David Aguilar
In-Reply-To: <1475159493.2435.7.camel@sapo.pt>
Vasco Almeida <vascomalmeida@sapo.pt> writes:
> On the other hand, would it make sense to translate these commands? If
> so, we would mark for translation the commands name of @cmd in
> main_loop().
>
> sub main_loop {
> - my @cmd = ([ 'status', \&status_cmd, ],
> - [ 'update', \&update_cmd, ],
> - [ 'revert', \&revert_cmd, ],
> - [ 'add untracked', \&add_untracked_cmd, ],
> - [ 'patch', \&patch_update_cmd, ],
> - [ 'diff', \&diff_cmd, ],
> - [ 'quit', \&quit_cmd, ],
> - [ 'help', \&help_cmd, ],
> + my @cmd = ([ __('status'), \&status_cmd, ],
> + [ __('update'), \&update_cmd, ],
> + [ __('revert'), \&revert_cmd, ],
> + [ __('add untracked'), \&add_untracked_cmd, ],
> + [ __('patch'), \&patch_update_cmd, ],
> + [ __('diff'), \&diff_cmd, ],
> + [ __('quit'), \&quit_cmd, ],
> + [ __('help'), \&help_cmd, ],
I don't know offhand. If the code to prompt and accept the command
given by the user can take the translated word (or a prefix of it),
theoretically I would say it could be made to work, but to me it is
dubious the benefit outweighs its downsides. It would make teaching
Git and troubleshooting over the phone harder, I would guess.
A: "Hi, I am in a 'git add -i' session."
B: "Give 's' at the prompt."
A: "My Git does not seem to take 's' as a valid command."
B: "What? I've never seen that problem."
... back and forth wastes 10 minutes ...
A: "By the way, I am running Git in Portuguese."
;-)
^ permalink raw reply
* Re: [PATCH v8 00/11] Git filter protocol
From: Junio C Hamano @ 2016-09-29 16:57 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Lars Schneider, git, Jeff King, Stefan Beller,
Jakub Narębski, Martin-Louis Bright, ramsay
In-Reply-To: <f7a4f828-bb1d-0ffa-e369-3b4fa476d9e5@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
>> 1) Git exits
>> 2) The filter process receives EOF and prints "STOP" to the log
>> 3) t0021 checks the content of the log
>>
>> Sometimes 3 happened before 2 which makes the test fail.
>> (Example: https://travis-ci.org/git/git/jobs/162660563 )
>>
>> I added a this to wait until the filter process terminates:
>>
>> +wait_for_filter_termination () {
>> + while ps | grep -v grep | grep -F "/t0021/rot13-filter.pl" >/dev/null 2>&1
>> + do
>> + echo "Waiting for /t0021/rot13-filter.pl to finish..."
>> + sleep 1
>> + done
>> +}
>>
>> Does this look OK to you?
> Do we need the ps at all ?
> How about this:
>
> +wait_for_filter_termination () {
> + while ! grep "STOP" LOGFILENAME >/dev/null
> + do
> + echo "Waiting for /t0021/rot13-filter.pl to finish..."
> + sleep 1
> + done
> +}
Running "ps" and grepping for a command is not suitable for script
to reliably tell things, so it is out of question. Compared to
that, your version looks slightly better, but what if the machinery
that being tested, i.e. the part that drives the filter process, is
buggy or becomes buggy and causes the filter process that writes
"STOP" to die before it actually writes that string?
I have a feeling that the machinery being tested needs to be fixed
so that the sequence is always be:
0) Git spawns the filter process, as it needs some contents to
be filtered.
1) Git did everything it needed to do and decides that is time
to go.
2) Filter process receives EOF and prints "STOP" to the log.
3) Git waits until the filter process finishes.
4) t0021, after Git finishes, checks the log.
Repeated sleep combined with grep is probably just sweeping the real
problem under the rug. Do we have enough information to do the
above?
An inspiration may be in the way we centrally clean all tempfiles
and lockfiles before exiting. We have a central registry of these
files that need cleaning up and have a single atexit(3) handler to
clean them up. Perhaps we need a registry that filter processes
spawned by the mechanism Lars introduces in this series, and have an
atexit(3) handler that closes the pipe to them (which signals the
filters that it is time for them to go) and wait(2) on them, or
something? I do not think we want any kill(2) to be involved in
this clean-up procedure, but I do think we should wait(2) on what we
spawn, as long as these processes are meant to be shut down when the
main process of Git exits (this is different from things like
credential-cache daemon where they are expected to persist and meant
to serve multiple Git processes).
^ permalink raw reply
* [PATCH 3/3] remove unnecessary check before QSORT
From: René Scharfe @ 2016-09-29 15:29 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
In-Reply-To: <67bddc37-4ee2-fef0-c852-e32645421e4c@web.de>
Add a semantic patch for removing checks similar to the one that QSORT
already does internally and apply it to the code base.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
builtin/fmt-merge-msg.c | 10 ++++------
contrib/coccinelle/qsort.cocci | 18 ++++++++++++++++++
sh-i18n--envsubst.c | 3 +--
3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 4976967..efab62f 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -314,12 +314,10 @@ static void add_people_info(struct strbuf *out,
struct string_list *authors,
struct string_list *committers)
{
- if (authors->nr)
- QSORT(authors->items, authors->nr,
- cmp_string_list_util_as_integral);
- if (committers->nr)
- QSORT(committers->items, committers->nr,
- cmp_string_list_util_as_integral);
+ QSORT(authors->items, authors->nr,
+ cmp_string_list_util_as_integral);
+ QSORT(committers->items, committers->nr,
+ cmp_string_list_util_as_integral);
credit_people(out, authors, 'a');
credit_people(out, committers, 'c');
diff --git a/contrib/coccinelle/qsort.cocci b/contrib/coccinelle/qsort.cocci
index a094e7c..22b93a9 100644
--- a/contrib/coccinelle/qsort.cocci
+++ b/contrib/coccinelle/qsort.cocci
@@ -17,3 +17,21 @@ expression nmemb, compar;
@@
- qsort(base, nmemb, sizeof(T), compar);
+ QSORT(base, nmemb, compar);
+
+@@
+expression base, nmemb, compar;
+@@
+- if (nmemb)
+ QSORT(base, nmemb, compar);
+
+@@
+expression base, nmemb, compar;
+@@
+- if (nmemb > 0)
+ QSORT(base, nmemb, compar);
+
+@@
+expression base, nmemb, compar;
+@@
+- if (nmemb > 1)
+ QSORT(base, nmemb, compar);
diff --git a/sh-i18n--envsubst.c b/sh-i18n--envsubst.c
index 3637a2a..c3a2b5a 100644
--- a/sh-i18n--envsubst.c
+++ b/sh-i18n--envsubst.c
@@ -230,8 +230,7 @@ cmp_string (const void *pstr1, const void *pstr2)
static inline void
string_list_sort (string_list_ty *slp)
{
- if (slp->nitems > 0)
- QSORT(slp->item, slp->nitems, cmp_string);
+ QSORT(slp->item, slp->nitems, cmp_string);
}
/* Test whether a sorted string list contains a given string. */
--
2.10.0
^ permalink raw reply related
* [PATCH 2/3] use QSORT
From: René Scharfe @ 2016-09-29 15:27 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
In-Reply-To: <67bddc37-4ee2-fef0-c852-e32645421e4c@web.de>
Apply the semantic patch contrib/coccinelle/qsort.cocci to the code
base, replacing calls of qsort(3) with QSORT. The resulting code is
shorter and supports empty arrays with NULL pointers.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
Freshly generated using coccicheck, compiles, survives make test.
bisect.c | 2 +-
builtin/describe.c | 2 +-
builtin/fast-export.c | 2 +-
builtin/fmt-merge-msg.c | 6 ++----
builtin/index-pack.c | 8 +++-----
builtin/mktree.c | 2 +-
builtin/name-rev.c | 3 +--
builtin/pack-objects.c | 7 +++----
builtin/remote.c | 3 +--
diff.c | 6 +++---
diffcore-delta.c | 5 +----
diffcore-order.c | 2 +-
diffcore-rename.c | 2 +-
dir.c | 4 ++--
fast-import.c | 4 ++--
fetch-pack.c | 2 +-
help.c | 15 +++++----------
line-log.c | 2 +-
pack-bitmap-write.c | 3 +--
pack-check.c | 2 +-
pack-write.c | 3 +--
pathspec.c | 3 +--
ref-filter.c | 2 +-
refs/files-backend.c | 2 +-
server-info.c | 2 +-
sh-i18n--envsubst.c | 2 +-
sha1-array.c | 2 +-
string-list.c | 2 +-
t/helper/test-dump-untracked-cache.c | 6 ++----
tree.c | 3 +--
30 files changed, 44 insertions(+), 65 deletions(-)
diff --git a/bisect.c b/bisect.c
index 6f512c2..21bc6da 100644
--- a/bisect.c
+++ b/bisect.c
@@ -215,7 +215,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
array[cnt].distance = distance;
cnt++;
}
- qsort(array, cnt, sizeof(*array), compare_commit_dist);
+ QSORT(array, cnt, compare_commit_dist);
for (p = list, i = 0; i < cnt; i++) {
char buf[100]; /* enough for dist=%d */
struct object *obj = &(array[i].commit->object);
diff --git a/builtin/describe.c b/builtin/describe.c
index 8a25abe..01490a1 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -352,7 +352,7 @@ static void describe(const char *arg, int last_one)
oid_to_hex(oid));
}
- qsort(all_matches, match_cnt, sizeof(all_matches[0]), compare_pt);
+ QSORT(all_matches, match_cnt, compare_pt);
if (gave_up_on) {
commit_list_insert_by_date(gave_up_on, &list);
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index c0652a7..1e815b5 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -347,7 +347,7 @@ static void show_filemodify(struct diff_queue_struct *q,
* Handle files below a directory first, in case they are all deleted
* and the directory changes to a file or symlink.
*/
- qsort(q->queue, q->nr, sizeof(q->queue[0]), depth_first);
+ QSORT(q->queue, q->nr, depth_first);
for (i = 0; i < q->nr; i++) {
struct diff_filespec *ospec = q->queue[i]->one;
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index dc2e9e4..4976967 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -315,12 +315,10 @@ static void add_people_info(struct strbuf *out,
struct string_list *committers)
{
if (authors->nr)
- qsort(authors->items,
- authors->nr, sizeof(authors->items[0]),
+ QSORT(authors->items, authors->nr,
cmp_string_list_util_as_integral);
if (committers->nr)
- qsort(committers->items,
- committers->nr, sizeof(committers->items[0]),
+ QSORT(committers->items, committers->nr,
cmp_string_list_util_as_integral);
credit_people(out, authors, 'a');
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 4a8b4ae..7657d0a 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1190,10 +1190,8 @@ static void resolve_deltas(void)
return;
/* Sort deltas by base SHA1/offset for fast searching */
- qsort(ofs_deltas, nr_ofs_deltas, sizeof(struct ofs_delta_entry),
- compare_ofs_delta_entry);
- qsort(ref_deltas, nr_ref_deltas, sizeof(struct ref_delta_entry),
- compare_ref_delta_entry);
+ QSORT(ofs_deltas, nr_ofs_deltas, compare_ofs_delta_entry);
+ QSORT(ref_deltas, nr_ref_deltas, compare_ref_delta_entry);
if (verbose || show_resolving_progress)
progress = start_progress(_("Resolving deltas"),
@@ -1356,7 +1354,7 @@ static void fix_unresolved_deltas(struct sha1file *f)
ALLOC_ARRAY(sorted_by_pos, nr_ref_deltas);
for (i = 0; i < nr_ref_deltas; i++)
sorted_by_pos[i] = &ref_deltas[i];
- qsort(sorted_by_pos, nr_ref_deltas, sizeof(*sorted_by_pos), delta_pos_compare);
+ QSORT(sorted_by_pos, nr_ref_deltas, delta_pos_compare);
for (i = 0; i < nr_ref_deltas; i++) {
struct ref_delta_entry *d = sorted_by_pos[i];
diff --git a/builtin/mktree.c b/builtin/mktree.c
index 4282b62..de9b40f 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -46,7 +46,7 @@ static void write_tree(unsigned char *sha1)
size_t size;
int i;
- qsort(entries, used, sizeof(*entries), ent_compare);
+ QSORT(entries, used, ent_compare);
for (size = i = 0; i < used; i++)
size += 32 + entries[i]->len;
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 57be35f..cd89d48 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -195,8 +195,7 @@ static const char *get_exact_ref_match(const struct object *o)
return NULL;
if (!tip_table.sorted) {
- qsort(tip_table.table, tip_table.nr, sizeof(*tip_table.table),
- tipcmp);
+ QSORT(tip_table.table, tip_table.nr, tipcmp);
tip_table.sorted = 1;
}
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 166e52c..8aeba6a 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1535,7 +1535,7 @@ static void get_object_details(void)
sorted_by_offset = xcalloc(to_pack.nr_objects, sizeof(struct object_entry *));
for (i = 0; i < to_pack.nr_objects; i++)
sorted_by_offset[i] = to_pack.objects + i;
- qsort(sorted_by_offset, to_pack.nr_objects, sizeof(*sorted_by_offset), pack_offset_sort);
+ QSORT(sorted_by_offset, to_pack.nr_objects, pack_offset_sort);
for (i = 0; i < to_pack.nr_objects; i++) {
struct object_entry *entry = sorted_by_offset[i];
@@ -2257,7 +2257,7 @@ static void prepare_pack(int window, int depth)
if (progress)
progress_state = start_progress(_("Compressing objects"),
nr_deltas);
- qsort(delta_list, n, sizeof(*delta_list), type_size_sort);
+ QSORT(delta_list, n, type_size_sort);
ll_find_deltas(delta_list, n, window+1, depth, &nr_done);
stop_progress(&progress_state);
if (nr_done != nr_deltas)
@@ -2449,8 +2449,7 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs)
}
if (in_pack.nr) {
- qsort(in_pack.array, in_pack.nr, sizeof(in_pack.array[0]),
- ofscmp);
+ QSORT(in_pack.array, in_pack.nr, ofscmp);
for (i = 0; i < in_pack.nr; i++) {
struct object *o = in_pack.array[i].object;
add_object_entry(o->oid.hash, o->type, "", 0);
diff --git a/builtin/remote.c b/builtin/remote.c
index 9f6a6b3..e52cf39 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1197,8 +1197,7 @@ static int show(int argc, const char **argv)
info.width = info.width2 = 0;
for_each_string_list(&states.push, add_push_to_show_info, &info);
- qsort(info.list->items, info.list->nr,
- sizeof(*info.list->items), cmp_string_with_push);
+ QSORT(info.list->items, info.list->nr, cmp_string_with_push);
if (info.list->nr)
printf_ln(Q_(" Local ref configured for 'git push'%s:",
" Local refs configured for 'git push'%s:",
diff --git a/diff.c b/diff.c
index a178ed3..c2f09fb 100644
--- a/diff.c
+++ b/diff.c
@@ -2019,7 +2019,7 @@ static void show_dirstat(struct diff_options *options)
return;
/* Show all directories with more than x% of the changes */
- qsort(dir.files, dir.nr, sizeof(dir.files[0]), dirstat_compare);
+ QSORT(dir.files, dir.nr, dirstat_compare);
gather_dirstat(options, &dir, changed, "", 0);
}
@@ -2063,7 +2063,7 @@ static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *o
return;
/* Show all directories with more than x% of the changes */
- qsort(dir.files, dir.nr, sizeof(dir.files[0]), dirstat_compare);
+ QSORT(dir.files, dir.nr, dirstat_compare);
gather_dirstat(options, &dir, changed, "", 0);
}
@@ -4923,7 +4923,7 @@ static int diffnamecmp(const void *a_, const void *b_)
void diffcore_fix_diff_index(struct diff_options *options)
{
struct diff_queue_struct *q = &diff_queued_diff;
- qsort(q->queue, q->nr, sizeof(q->queue[0]), diffnamecmp);
+ QSORT(q->queue, q->nr, diffnamecmp);
}
void diffcore_std(struct diff_options *options)
diff --git a/diffcore-delta.c b/diffcore-delta.c
index 4159748..2ebedb3 100644
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -158,10 +158,7 @@ static struct spanhash_top *hash_chars(struct diff_filespec *one)
n = 0;
accum1 = accum2 = 0;
}
- qsort(hash->data,
- 1ul << hash->alloc_log2,
- sizeof(hash->data[0]),
- spanhash_cmp);
+ QSORT(hash->data, 1ul << hash->alloc_log2, spanhash_cmp);
return hash;
}
diff --git a/diffcore-order.c b/diffcore-order.c
index 69d41f7..1957f82 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -101,7 +101,7 @@ void order_objects(const char *orderfile, obj_path_fn_t obj_path,
objs[i].orig_order = i;
objs[i].order = match_order(obj_path(objs[i].obj));
}
- qsort(objs, nr, sizeof(*objs), compare_objs_order);
+ QSORT(objs, nr, compare_objs_order);
}
static const char *pair_pathtwo(void *obj)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 73d003a..54a2396 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -580,7 +580,7 @@ void diffcore_rename(struct diff_options *options)
stop_progress(&progress);
/* cost matrix sorted by most to least similar pair */
- qsort(mx, dst_cnt * NUM_CANDIDATE_PER_DST, sizeof(*mx), score_compare);
+ QSORT(mx, dst_cnt * NUM_CANDIDATE_PER_DST, score_compare);
rename_count += find_renames(mx, dst_cnt, minimum_score, 0);
if (detect_rename == DIFF_DETECT_COPY)
diff --git a/dir.c b/dir.c
index 9e09bcb..3bad1ad 100644
--- a/dir.c
+++ b/dir.c
@@ -2005,8 +2005,8 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru
if (!len || treat_leading_path(dir, path, len, simplify))
read_directory_recursive(dir, path, len, untracked, 0, simplify);
free_simplify(simplify);
- qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);
- qsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), cmp_name);
+ QSORT(dir->entries, dir->nr, cmp_name);
+ QSORT(dir->ignored, dir->ignored_nr, cmp_name);
if (dir->untracked) {
static struct trace_key trace_untracked_stats = TRACE_KEY_INIT(UNTRACKED_STATS);
trace_printf_key(&trace_untracked_stats,
diff --git a/fast-import.c b/fast-import.c
index bf53ac9..cb545d7 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1460,9 +1460,9 @@ static void mktree(struct tree_content *t, int v, struct strbuf *b)
unsigned int i;
if (!v)
- qsort(t->entries,t->entry_count,sizeof(t->entries[0]),tecmp0);
+ QSORT(t->entries, t->entry_count, tecmp0);
else
- qsort(t->entries,t->entry_count,sizeof(t->entries[0]),tecmp1);
+ QSORT(t->entries, t->entry_count, tecmp1);
for (i = 0; i < t->entry_count; i++) {
if (t->entries[i]->versions[v].mode)
diff --git a/fetch-pack.c b/fetch-pack.c
index 85e77af..8a38d30 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -812,7 +812,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
int agent_len;
sort_ref_list(&ref, ref_compare_name);
- qsort(sought, nr_sought, sizeof(*sought), cmp_ref_by_name);
+ QSORT(sought, nr_sought, cmp_ref_by_name);
if ((args->depth > 0 || is_repository_shallow()) && !server_supports("shallow"))
die("Server does not support shallow clients");
diff --git a/help.c b/help.c
index 2ff3b5a..53e2a67 100644
--- a/help.c
+++ b/help.c
@@ -170,8 +170,7 @@ void load_command_list(const char *prefix,
if (exec_path) {
list_commands_in_dir(main_cmds, exec_path, prefix);
- qsort(main_cmds->names, main_cmds->cnt,
- sizeof(*main_cmds->names), cmdname_compare);
+ QSORT(main_cmds->names, main_cmds->cnt, cmdname_compare);
uniq(main_cmds);
}
@@ -190,8 +189,7 @@ void load_command_list(const char *prefix,
}
free(paths);
- qsort(other_cmds->names, other_cmds->cnt,
- sizeof(*other_cmds->names), cmdname_compare);
+ QSORT(other_cmds->names, other_cmds->cnt, cmdname_compare);
uniq(other_cmds);
}
exclude_cmds(other_cmds, main_cmds);
@@ -238,8 +236,7 @@ void list_common_cmds_help(void)
longest = strlen(common_cmds[i].name);
}
- qsort(common_cmds, ARRAY_SIZE(common_cmds),
- sizeof(common_cmds[0]), cmd_group_cmp);
+ QSORT(common_cmds, ARRAY_SIZE(common_cmds), cmd_group_cmp);
puts(_("These are common Git commands used in various situations:"));
@@ -324,8 +321,7 @@ const char *help_unknown_cmd(const char *cmd)
add_cmd_list(&main_cmds, &aliases);
add_cmd_list(&main_cmds, &other_cmds);
- qsort(main_cmds.names, main_cmds.cnt,
- sizeof(*main_cmds.names), cmdname_compare);
+ QSORT(main_cmds.names, main_cmds.cnt, cmdname_compare);
uniq(&main_cmds);
/* This abuses cmdname->len for levenshtein distance */
@@ -359,8 +355,7 @@ const char *help_unknown_cmd(const char *cmd)
levenshtein(cmd, candidate, 0, 2, 1, 3) + 1;
}
- qsort(main_cmds.names, main_cmds.cnt,
- sizeof(*main_cmds.names), levenshtein_compare);
+ QSORT(main_cmds.names, main_cmds.cnt, levenshtein_compare);
if (!main_cmds.cnt)
die(_("Uh oh. Your system reports no Git commands at all."));
diff --git a/line-log.c b/line-log.c
index 916e724..65f3558 100644
--- a/line-log.c
+++ b/line-log.c
@@ -113,7 +113,7 @@ void sort_and_merge_range_set(struct range_set *rs)
int i;
int o = 0; /* output cursor */
- qsort(rs->ranges, rs->nr, sizeof(struct range), range_cmp);
+ QSORT(rs->ranges, rs->nr, range_cmp);
for (i = 0; i < rs->nr; i++) {
if (rs->ranges[i].start == rs->ranges[i].end)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index c30bcd0..9705596 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -385,8 +385,7 @@ void bitmap_writer_select_commits(struct commit **indexed_commits,
{
unsigned int i = 0, j, next;
- qsort(indexed_commits, indexed_commits_nr, sizeof(indexed_commits[0]),
- date_compare);
+ QSORT(indexed_commits, indexed_commits_nr, date_compare);
if (writer.show_progress)
writer.progress = start_progress("Selecting bitmap commits", 0);
diff --git a/pack-check.c b/pack-check.c
index d123846..72440a8 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -99,7 +99,7 @@ static int verify_packfile(struct packed_git *p,
entries[i].offset = nth_packed_object_offset(p, i);
entries[i].nr = i;
}
- qsort(entries, nr_objects, sizeof(*entries), compare_entries);
+ QSORT(entries, nr_objects, compare_entries);
for (i = 0; i < nr_objects; i++) {
void *data;
diff --git a/pack-write.c b/pack-write.c
index ea0b788..88bc7f9 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -61,8 +61,7 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec
if (objects[i]->offset > last_obj_offset)
last_obj_offset = objects[i]->offset;
}
- qsort(sorted_by_sha, nr_objects, sizeof(sorted_by_sha[0]),
- sha1_compare);
+ QSORT(sorted_by_sha, nr_objects, sha1_compare);
}
else
sorted_by_sha = list = last = NULL;
diff --git a/pathspec.c b/pathspec.c
index 24e0dd5..eda13b5 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -446,8 +446,7 @@ void parse_pathspec(struct pathspec *pathspec,
if (pathspec->magic & PATHSPEC_MAXDEPTH) {
if (flags & PATHSPEC_KEEP_ORDER)
die("BUG: PATHSPEC_MAXDEPTH_VALID and PATHSPEC_KEEP_ORDER are incompatible");
- qsort(pathspec->items, pathspec->nr,
- sizeof(struct pathspec_item), pathspec_item_cmp);
+ QSORT(pathspec->items, pathspec->nr, pathspec_item_cmp);
}
}
diff --git a/ref-filter.c b/ref-filter.c
index 9adbb8a..44029b0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1573,7 +1573,7 @@ static int compare_refs(const void *a_, const void *b_)
void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
{
ref_sorting = sorting;
- qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
+ QSORT(array->items, array->nr, compare_refs);
}
static void append_literal(const char *cp, const char *ep, struct ref_formatting_state *state)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0709f60..d16feb1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -501,7 +501,7 @@ static void sort_ref_dir(struct ref_dir *dir)
if (dir->sorted == dir->nr)
return;
- qsort(dir->entries, dir->nr, sizeof(*dir->entries), ref_entry_cmp);
+ QSORT(dir->entries, dir->nr, ref_entry_cmp);
/* Remove any duplicates: */
for (i = 0, j = 0; j < dir->nr; j++) {
diff --git a/server-info.c b/server-info.c
index 75dd677..7bc4e75 100644
--- a/server-info.c
+++ b/server-info.c
@@ -229,7 +229,7 @@ static void init_pack_info(const char *infofile, int force)
}
/* renumber them */
- qsort(info, num_pack, sizeof(info[0]), compare_info);
+ QSORT(info, num_pack, compare_info);
for (i = 0; i < num_pack; i++)
info[i]->new_num = i;
}
diff --git a/sh-i18n--envsubst.c b/sh-i18n--envsubst.c
index e06b2c1..3637a2a 100644
--- a/sh-i18n--envsubst.c
+++ b/sh-i18n--envsubst.c
@@ -231,7 +231,7 @@ static inline void
string_list_sort (string_list_ty *slp)
{
if (slp->nitems > 0)
- qsort (slp->item, slp->nitems, sizeof (slp->item[0]), cmp_string);
+ QSORT(slp->item, slp->nitems, cmp_string);
}
/* Test whether a sorted string list contains a given string. */
diff --git a/sha1-array.c b/sha1-array.c
index 6f4a224..21188de 100644
--- a/sha1-array.c
+++ b/sha1-array.c
@@ -16,7 +16,7 @@ static int void_hashcmp(const void *a, const void *b)
static void sha1_array_sort(struct sha1_array *array)
{
- qsort(array->sha1, array->nr, sizeof(*array->sha1), void_hashcmp);
+ QSORT(array->sha1, array->nr, void_hashcmp);
array->sorted = 1;
}
diff --git a/string-list.c b/string-list.c
index 62d2084..8c83cac 100644
--- a/string-list.c
+++ b/string-list.c
@@ -225,7 +225,7 @@ static int cmp_items(const void *a, const void *b)
void string_list_sort(struct string_list *list)
{
compare_for_qsort = list->cmp ? list->cmp : strcmp;
- qsort(list->items, list->nr, sizeof(*list->items), cmp_items);
+ QSORT(list->items, list->nr, cmp_items);
}
struct string_list_item *unsorted_string_list_lookup(struct string_list *list,
diff --git a/t/helper/test-dump-untracked-cache.c b/t/helper/test-dump-untracked-cache.c
index 50112cc..f752532 100644
--- a/t/helper/test-dump-untracked-cache.c
+++ b/t/helper/test-dump-untracked-cache.c
@@ -18,10 +18,8 @@ static int compare_dir(const void *a_, const void *b_)
static void dump(struct untracked_cache_dir *ucd, struct strbuf *base)
{
int i, len;
- qsort(ucd->untracked, ucd->untracked_nr, sizeof(*ucd->untracked),
- compare_untracked);
- qsort(ucd->dirs, ucd->dirs_nr, sizeof(*ucd->dirs),
- compare_dir);
+ QSORT(ucd->untracked, ucd->untracked_nr, compare_untracked);
+ QSORT(ucd->dirs, ucd->dirs_nr, compare_dir);
len = base->len;
strbuf_addf(base, "%s/", ucd->name);
printf("%s %s", base->buf,
diff --git a/tree.c b/tree.c
index 2b5a5a8..ce345c5 100644
--- a/tree.c
+++ b/tree.c
@@ -180,8 +180,7 @@ int read_tree(struct tree *tree, int stage, struct pathspec *match)
* Sort the cache entry -- we need to nuke the cache tree, though.
*/
cache_tree_free(&active_cache_tree);
- qsort(active_cache, active_nr, sizeof(active_cache[0]),
- cmp_cache_name_compare);
+ QSORT(active_cache, active_nr, cmp_cache_name_compare);
return 0;
}
--
2.10.0
^ permalink raw reply related
* [PATCH 1/3] add QSORT
From: René Scharfe @ 2016-09-29 15:23 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
Add the macro QSORT, a convenient wrapper for qsort(3) that infers the
size of the array elements and supports the convention of initializing
empty arrays with a NULL pointer, which we use in some places.
Calling qsort(3) directly with a NULL pointer is undefined -- even with
an element count of zero -- and allows the compiler to optimize away any
following NULL checks. Using the macro avoids such surprises.
Add a semantic patch as well to demonstrate the macro's usage and to
automate the transformation of trivial cases.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
contrib/coccinelle/qsort.cocci | 19 +++++++++++++++++++
git-compat-util.h | 8 ++++++++
2 files changed, 27 insertions(+)
create mode 100644 contrib/coccinelle/qsort.cocci
diff --git a/contrib/coccinelle/qsort.cocci b/contrib/coccinelle/qsort.cocci
new file mode 100644
index 0000000..a094e7c
--- /dev/null
+++ b/contrib/coccinelle/qsort.cocci
@@ -0,0 +1,19 @@
+@@
+expression base, nmemb, compar;
+@@
+- qsort(base, nmemb, sizeof(*base), compar);
++ QSORT(base, nmemb, compar);
+
+@@
+expression base, nmemb, compar;
+@@
+- qsort(base, nmemb, sizeof(base[0]), compar);
++ QSORT(base, nmemb, compar);
+
+@@
+type T;
+T *base;
+expression nmemb, compar;
+@@
+- qsort(base, nmemb, sizeof(T), compar);
++ QSORT(base, nmemb, compar);
diff --git a/git-compat-util.h b/git-compat-util.h
index 8aab0c3..d7ed137 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -977,6 +977,14 @@ void git_qsort(void *base, size_t nmemb, size_t size,
#define qsort git_qsort
#endif
+#define QSORT(base, n, compar) sane_qsort((base), (n), sizeof(*(base)), compar)
+static void inline sane_qsort(void *base, size_t nmemb, size_t size,
+ int(*compar)(const void *, const void *))
+{
+ if (nmemb > 1)
+ qsort(base, nmemb, size, compar);
+}
+
#ifndef REG_STARTEND
#error "Git requires REG_STARTEND support. Compile with NO_REGEX=NeedsStartEnd"
#endif
--
2.10.0
^ permalink raw reply related
* Re: [PATCH 10/10] get_short_sha1: list ambiguous objects on error
From: Jeff King @ 2016-09-29 14:55 UTC (permalink / raw)
To: Kyle J. McKay; +Cc: Linus Torvalds, Junio C Hamano, Git Mailing List
In-Reply-To: <2FECD796-7B92-41BB-A0AF-57650FF7E78D@gmail.com>
On Thu, Sep 29, 2016 at 07:36:27AM -0700, Kyle J. McKay wrote:
> On Sep 29, 2016, at 06:24, Jeff King wrote:
>
> > > If you are doing "git show 235234" it should pick the tag (if it
> > > peels to a
> > > committish) because Git has already set a precedent of preferring
> > > tags over
> > > commits when it disambiguates ref names and otherwise pick the
> > > commit.
> >
> > I'm not convinced that picking the tag is actually helpful in this case;
> > I agree with Linus that feeding something to "git show" almost always
> > wants to choose the commit.
>
> Since "git show" peels tags you end up seeing the commit it refers to
> (assuming it's a committish tag).
Yes, but it's almost certainly _not_ the commit you meant. From your
example:
> c512b03:
> c512b035556eff4d commit Merge branch 'rc/maint-reflog-msg-for-forced
> c512b0344196931a tag (v0.99.9a) GIT 0.99.9a
If I'm looking for the commit c512b03, then it almost certainly isn't
v0.99.9a. That tag's commit is e634aec. Or another way of thinking about
it: you want to guess what the _writer_ of the note meant. Why would
somebody write "c512b03" when they could have written "v0.99.9a"? And
they certainly would not have written it if they meant "e634aec". :)
> > I also don't think tag ambiguity in short sha1s is all that interesting.
>
> The Linux repository has this:
>
> 901069c:
> 901069c71415a76d commit iwlagn: change Copyright to 2011
> 901069c5c5b15532 tag (v2.6.38-rc4) Linux 2.6.38-rc4
Sure, I'm not surprised there's a collision. But I'd expect those to be
a tiny fraction of collisions. Here's the breakdown of object types in
my clone of linux.git:
$ git cat-file --batch-all-objects --batch-check='%(objecttype)' |
sort | uniq -c
1421198 blob
618073 commit
479 tag
2877913 tree
That's a hundredth of a percent tag objects. The chance that you have
_a_ 7-hex collision with a tag is relatively high. But the chance that
any given collision involves a tag is rather small.
-Peff
^ permalink raw reply
* Re: [PATCH 10/10] get_short_sha1: list ambiguous objects on error
From: Kyle J. McKay @ 2016-09-29 14:36 UTC (permalink / raw)
To: Jeff King; +Cc: Linus Torvalds, Junio C Hamano, Git Mailing List
In-Reply-To: <20160929132425.of7m5t4tsqcb6bbk@sigill.intra.peff.net>
On Sep 29, 2016, at 06:24, Jeff King wrote:
>> If you are doing "git show 235234" it should pick the tag (if it
>> peels to a
>> committish) because Git has already set a precedent of preferring
>> tags over
>> commits when it disambiguates ref names and otherwise pick the
>> commit.
>
> I'm not convinced that picking the tag is actually helpful in this
> case;
> I agree with Linus that feeding something to "git show" almost always
> wants to choose the commit.
Since "git show" peels tags you end up seeing the commit it refers to
(assuming it's a committish tag).
> I also don't think tag ambiguity in short sha1s is all that
> interesting.
The Linux repository has this:
901069c:
901069c71415a76d commit iwlagn: change Copyright to 2011
901069c5c5b15532 tag (v2.6.38-rc4) Linux 2.6.38-rc4
Since that tag peels to a commit, it seems like it would be incorrect
to pick the commit over the tag when you're looking for a committish.
Either 901069c should resolve to the tag (which gets peeled to the
commit) or it should error out with the hint messages.
The Git repository has this:
c512b03:
c512b035556eff4d commit Merge branch 'rc/maint-reflog-msg-for-
forced
c512b0344196931a tag (v0.99.9a) GIT 0.99.9a
So perhaps it's a little bit more interesting than it first appears. :)
--Kyle
^ permalink raw reply
* Re: [PATCH v2 02/11] i18n: add--interactive: mark simple here documents for translation
From: Vasco Almeida @ 2016-09-29 14:31 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
David Aguilar
In-Reply-To: <xmqqmvivy4i9.fsf@gitster.mtv.corp.google.com>
A Dom, 25-09-2016 às 15:54 -0700, Junio C Hamano escreveu:
> > sub status_cmd {
> > @@ -1573,14 +1573,14 @@ sub quit_cmd {
> > }
> >
> > sub help_cmd {
> > - print colored $help_color, <<\EOF ;
> > -status - show paths with changes
> > + print colored $help_color, __(
> > +"status - show paths with changes
> > update - add working tree state to the staged set of
> changes
> > revert - revert staged set of changes back to the HEAD
> version
> > patch - pick hunks and update selectively
> > diff - view diff between HEAD and index
> > -add untracked - add contents of untracked files to the staged set
> of changes
> > -EOF
> > +add untracked - add contents of untracked files to the staged set
> of changes"),
> > +"\n";
> > }
>
> Do we need TRANSLATORS: comment to all of the above not to touch the
> command words that are explained and translate only the explanation?
Yes, it is better to have that comment.
On the other hand, would it make sense to translate these commands? If
so, we would mark for translation the commands name of @cmd in
main_loop().
sub main_loop {
- my @cmd = ([ 'status', \&status_cmd, ],
- [ 'update', \&update_cmd, ],
- [ 'revert', \&revert_cmd, ],
- [ 'add untracked', \&add_untracked_cmd, ],
- [ 'patch', \&patch_update_cmd, ],
- [ 'diff', \&diff_cmd, ],
- [ 'quit', \&quit_cmd, ],
- [ 'help', \&help_cmd, ],
+ my @cmd = ([ __('status'), \&status_cmd, ],
+ [ __('update'), \&update_cmd, ],
+ [ __('revert'), \&revert_cmd, ],
+ [ __('add untracked'), \&add_untracked_cmd, ],
+ [ __('patch'), \&patch_update_cmd, ],
+ [ __('diff'), \&diff_cmd, ],
+ [ __('quit'), \&quit_cmd, ],
+ [ __('help'), \&help_cmd, ],
^ permalink raw reply
* Re: [PATCH 10/10] get_short_sha1: list ambiguous objects on error
From: Jeff King @ 2016-09-29 13:24 UTC (permalink / raw)
To: Kyle J. McKay; +Cc: Linus Torvalds, Junio C Hamano, Git Mailing List
In-Reply-To: <841D4FC2-9673-486A-8D94-8967188CCC60@gmail.com>
On Thu, Sep 29, 2016 at 06:01:51AM -0700, Kyle J. McKay wrote:
> But perhaps it makes sense to actually pick one if there's only one
> disambiguation of the type you're looking for.
>
> For example given:
>
> 235234a blob
> 2352347 tag
> 235234f tree
> 2352340 commit
>
> If you are doing "git cat-file blob 235234" it should pick the blob and spit
> out a warning (and similarly for other cat-file types). But "git cat-file
> -p 235234" would give the fatal error with the disambiguation hints because
> it wants type "any".
That code is already there; it's just a matter of whether git has enough
information to know the context. E.g. (in git.git):
$ git show b2e11
error: short SHA1 b2e11 is ambiguous
hint: The candidates are:
hint: b2e1196 tag v2.8.0-rc1
hint: b2e11d1 tree
...
$ git log b2e11
commit ab5d01a29eb7380ceab070f0807c2939849c44bc (tag: v2.8.0-rc1)
...
The "show" command can show anything, but "log" really wants
committishes, so it's able to disambiguate. It looks like cat-file never
learned to feed its context, but it's probably something like this:
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 94e67eb..ecbb959 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -56,12 +56,22 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
struct object_info oi = {NULL};
struct strbuf sb = STRBUF_INIT;
unsigned flags = LOOKUP_REPLACE_OBJECT;
+ unsigned sha1_flags = 0;
const char *path = force_path;
if (unknown_type)
flags |= LOOKUP_UNKNOWN_OBJECT;
- if (get_sha1_with_context(obj_name, 0, oid.hash, &obj_context))
+ if (exp_type) {
+ if (!strcmp(exp_type, "commit"))
+ sha1_flags |= GET_SHA1_COMMITTISH;
+ else if(!strcmp(exp_type, "tree"))
+ sha1_flags |= GET_SHA1_TREEISH;
+ else if(!strcmp(exp_type, "blob"))
+ sha1_flags |= GET_SHA1_BLOB;
+ }
+
+ if (get_sha1_with_context(obj_name, sha1_flags, oid.hash, &obj_context))
die("Not a valid object name %s", obj_name);
if (!path)
> If you are doing "git show 235234" it should pick the tag (if it peels to a
> committish) because Git has already set a precedent of preferring tags over
> commits when it disambiguates ref names and otherwise pick the commit.
I'm not convinced that picking the tag is actually helpful in this case;
I agree with Linus that feeding something to "git show" almost always
wants to choose the commit.
I also don't think tag ambiguity in short sha1s is all that interesting.
There are a tiny number of tag objects. Most of your collisions are
going to be with trees or blobs, which should generally outnumber
commits by a factor of 5-10, though it depends on your workflow (git.git
does not have a deep tree, so it's only a factor of 4).
And if you just want to choose a committish over trees and blobs, well,
then; I invite you to check out the core.disambiguate patch I sent
elsewhere in the thread. :)
-Peff
^ permalink raw reply related
* Re: [PATCH 10/10] get_short_sha1: list ambiguous objects on error
From: Jeff King @ 2016-09-29 13:03 UTC (permalink / raw)
To: Kyle J. McKay; +Cc: Junio C Hamano, Linus Torvalds, Git Mailing List
In-Reply-To: <2242637D-4C3B-4AF2-8BE4-823B3E1745D5@gmail.com>
On Thu, Sep 29, 2016 at 04:46:19AM -0700, Kyle J. McKay wrote:
> This hint: information is excellent. There needs to be a way to show it on
> demand.
>
> $ git rev-parse --disambiguate=b2e1
> b2e11962c5e6a9c81aa712c751c83a743fd4f384
> b2e11d1bb40c5f81a2f4e37b9f9a60ec7474eeab
> b2e163272c01aca4aee4684f5c683ba341c1953d
> b2e18954c03ff502053cb74d142faab7d2a8dacb
> b2e1895ca92ec2037349d88b945ba64ebf16d62d
>
> Not nearly so helpful, but the operation of --disambiguate cannot be changed
> without breaking current scripts.
>
> Can your excellent "hint:" output above be attached to the --disambiguate
> option somehow, please. Something like this perhaps:
>
> $ git rev-parse --disambiguate-list=b2e1
> b2e1196 tag v2.8.0-rc1
> b2e11d1 tree
> b2e1632 commit 2007-11-14 - Merge branch 'bs/maint-commit-options'
> b2e1759 blob
> b2e18954 blob
> b2e1895c blob
I think the "right" way to do this is pipe the list of sha1s into
another git commit which can format them however you want.
Unfortunately, there isn't a single command that does a great job:
- "cat-file --batch-check" can show you the sha1 and type, but it
won't abbreviate sha1s, and it won't show you commit/tag information
- "log --stdin --no-walk" will format the commit however you like, but
skips the trees and blobs entirely, and the tag can only be seen via
"%d"
- "for-each-ref" has flexible formatting, too, but wants to format
refs, not objects (and doesn't read from stdin).
IMHO that is a sign that our formatting tools aren't as good as they
could be (I think the right tool is cat-file, but it should be able to
do all of the formatting that the other commands can do).
Of course if you really just want human-readable output, then:
$ git cat-file -e b2e1
error: short SHA1 b2e1 is ambiguous
hint: The candidates are:
hint: b2e1196 tag v2.8.0-rc1
hint: b2e11d1 tree
hint: b2e1632 commit 2007-11-14 - Merge branch 'bs/maint-commit-options'
hint: b2e1759 blob
hint: b2e18954 blob
hint: b2e1895c blob
fatal: Not a valid object name b2e1
is pretty easy.
That being said, I don't mind if somebody wanted to do a rev-parse
option on top of my series. The formatting code is already split into
its own function.
-Peff
^ permalink raw reply
* Re: [PATCH 10/10] get_short_sha1: list ambiguous objects on error
From: Kyle J. McKay @ 2016-09-29 13:01 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jeff King, Junio C Hamano, Git Mailing List
In-Reply-To: <CA+55aFyfvvqq1c=hZcuL-yPavp2tjzx8r3bFJnMY7DAE7YcB=Q@mail.gmail.com>
On Sep 26, 2016, at 09:36, Linus Torvalds wrote:
> On Mon, Sep 26, 2016 at 5:00 AM, Jeff King <peff@peff.net> wrote:
>>
>> This patch teaches get_short_sha1() to list the sha1s of the
>> objects it found, along with a few bits of information that
>> may help the user decide which one they meant.
>
> This looks very good to me, but I wonder if it couldn't be even more
> aggressive.
>
> In particular, the only hashes that most people ever use in short form
> are commit hashes. Those are the ones you'd use in normal human
> interactions to point to something happening.
>
> So when the disambiguation notices that there is ambiguity, but there
> is only _one_ commit, maybe it should just have an aggressive mode
> that says "use that as if it wasn't ambiguous".
If you have this:
faa23ec9b437812ce2fc9a5b3d59418d672debc1 refs/heads/ambig
7f40afe646fa3f8a0f361b6f567d8f7d7a184c10 refs/tags/ambig
and you do this:
$ git rev-parse ambig
warning: refname 'ambig' is ambiguous.
7f40afe646fa3f8a0f361b6f567d8f7d7a184c10
Git automatically prefers the tag over the branch, but it does spit
out a warning.
> And then have an explicit command (or flag) to do disambiguation for
> when you explicitly want it.
I think you don't even need that. Git already does disambiguation for
ref names, picks one and spits out a warning.
Why not do the same for short hash names when it makes sense?
> Rationale: you'd never care about short forms for tags. You'd just use
> the tag name. And while blob ID's certainly show up in short form in
> diff output (in the "index" line), very few people will use them. And
> tree hashes are basically never seen outside of any plumbing commands
> and then seldom in shortened form.
>
> So I think it would make sense to default to a mode that just picks
> the commit hash if there is only one such hash. Sure, some command
> might want a "treeish", but a commit is still more likely than a tree
> or a tag.
>
> But regardless, this series looks like a good thing.
I like it too.
But perhaps it makes sense to actually pick one if there's only one
disambiguation of the type you're looking for.
For example given:
235234a blob
2352347 tag
235234f tree
2352340 commit
If you are doing "git cat-file blob 235234" it should pick the blob
and spit out a warning (and similarly for other cat-file types). But
"git cat-file -p 235234" would give the fatal error with the
disambiguation hints because it wants type "any".
If you are doing "git show 235234" it should pick the tag (if it peels
to a committish) because Git has already set a precedent of preferring
tags over commits when it disambiguates ref names and otherwise pick
the commit.
Lets consider this approach using the stats for the Linux kernel:
> Ambiguous prefix length 7 counts:
> prefixes: 44733
> objects: 89766
>
> Ambiguous length 11 (but not at length 12) info:
> prefixes: 2
> 0 (with 1 or more commit disambiguations)
>
> Ambiguous length 10 (but not at length 11) info:
> prefixes: 12
> 3 (with 1 or more commit disambiguations)
> 0 (with 2 or more commit disambiguations)
>
> Ambiguous length 9 (but not at length 10) info:
> prefixes: 186
> 43 (with 1 or more commit disambiguations)
> 1 (with 2 or more commit disambiguations)
>
> Ambiguous length 8 (but not at length 9) info:
> prefixes: 2723
> 651 (with 1 or more commit disambiguations)
> 40 (with 2 or more commit disambiguations)
>
> Ambiguous length 7 (but not at length 8) info:
> prefixes: 41864
> 9842 (with 1 or more commit disambiguations)
> 680 (with 2 or more commit disambiguations)
Of the 44733 ambiguous length 7 prefixes, only about 10539 of them
disambiguate into one or more commit objects.
But if we apply the "spit a warning and prefer a commit object if
there's only one and you're looking for a committish" rule, that drops
the number from 10539 to about 721. In other words, only about 7% of
the previously ambiguous short commit SHA1 prefixes would continue to
be ambiguous at length 7. In fact it almost makes a prefix length of
9 good enough, there's just the one at length 9 that disambiguates
into more than one commit (45f014c52).
--Kyle
^ permalink raw reply
* Re: Changing the default for "core.abbrev"?
From: Kyle J. McKay @ 2016-09-29 13:01 UTC (permalink / raw)
To: Junio C Hamano, Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <xmqq37knwcf4.fsf@gitster.mtv.corp.google.com>
On Sep 25, 2016, at 18:39, Linus Torvalds wrote:
> The kernel, these days, is at roughly 5 million objects, and while the
> seven hex digits are still often enough for uniqueness (and git will
> always add digits *until* it is unique), it's long been at the point
> where I tell people to do
>
> git config --global core.abbrev 12
>
> because even though git will extend the seven hex digits until the
> object name is unique, that only reflects the *current* situation in
> the repository. With 5 million objects and a very healthy growth rate,
> a 7-8 hex digit number that is unique today is not necessarily unique
> a month or two from now, and then it gets annoying when a commit
> message has a short git ID that is no longer unique when you go back
> and try to figure out what went wrong in that commit.
On Sep 25, 2016, at 20:46, Junio C Hamano wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> I can just keep reminding kernel maintainers and developers to update
>> their git config, but maybe it would be a good idea to just admit
>> that
>> the defaults picked in 2005 weren't necessarily the best ones
>> possible, and those could be bumped up a bit?
>
> I am not quite sure how good any new default would be, though. Just
> like any timeout is not long enough for somebody, growing projects
> will eventually hit whatever abbreviation length they start with.
This made me curious what the situation is really like. So I crunched
some data.
Using a recent clone of $korg/torvalds/linux:
$ git rev-parse --verify d597639e203
error: short SHA1 d597639e203 is ambiguous.
fatal: Needed a single revision
So the kernel already has 11-character "short" SHA1s that are
ambiguous. Is a core.abbrev setting of 12 really good enough?
Here are the stats on the kernel's repository:
Ambiguous length 11 (but not at length 12) info:
prefixes: 2
0 (with 1 or more commit disambiguations)
Ambiguous length 10 (but not at length 11) info:
prefixes: 12
3 (with 1 or more commit disambiguations)
0 (with 2 or more commit disambiguations)
Ambiguous length 9 (but not at length 10) info:
prefixes: 186
43 (with 1 or more commit disambiguations)
1 (with 2 or more commit disambiguations)
0 (with 3 or more disambiguations)
Ambiguous length 8 (but not at length 9) info:
prefixes: 2723
651 (with 1 or more commit disambiguations)
40 (with 2 or more commit disambiguations)
1 (with 3 or more disambiguations)
maxambig: 3 (there is 1 of them)
Ambiguous length 7 (but not at length 8) info:
prefixes: 41864
9842 (with 1 or more commit disambiguations)
680 (with 2 or more commit disambiguations)
299 (with 3 or more disambiguations)
maxambig: 3 (there are 299 of them)
The "maxambig" value is the maximum number of disambiguations for any
single prefix at that prefix length. So for prefixes of length 7
there are 299 that disambiguate into 3 objects.
Just out of curiosity, generating stats on the Git repository gives:
Ambiguous length 8 (but not at length 9) info:
prefixes: 7
3 (with 1 or more commit disambiguations)
2 (with 2 or more commit disambiguations)
0 (with 3 or more disambiguations)
Ambiguous length 7 (but not at length 8) info:
prefixes: 87
36 (with 1 or more commit disambiguations)
3 (with 2 or more commit disambiguations)
0 (with 3 or more disambiguations)
Running the stats on $github/gitster/git produces some ambiguous
length 9 prefixes (one of which contains a commit disambiguation).
--Kyle
^ permalink raw reply
* Re: [PATCH/RFC] git log --oneline alternative with dates, times and initials
From: Jeff King @ 2016-09-29 12:52 UTC (permalink / raw)
To: Kyle J. McKay; +Cc: Git mailing list, Junio C Hamano
In-Reply-To: <82EE6519-E58F-4382-87A5-55D9D1BBDCA9@gmail.com>
On Thu, Sep 29, 2016 at 04:00:06AM -0700, Kyle J. McKay wrote:
> > Each of those commits[1] needs some minor polish, and as I'm not really
> > that interested in fancy log output myself, I don't plan on working on
> > them further. I was mostly curious just how close we were. But if you'd
> > like to pursue it, feel free to use them as a starting point.
>
> Those patches are missing some of the features like showing root commits,
> handling two letter initials, showing the weekday, inserting a break where
> needed to avoid parent-child confusion in graph output and properly handling
> Duy's initials. :)
I'm not too surprised. I literally looked at the first screenshot from
your output and thought "surely git can do that with some minor tweaks".
Nor am I surprised that there are cases where the output is funny (99%
of the time I spent on it was tracking down that graph-padding bug).
I have no problem taking this in contrib or whatever, until a point when
Git is capable of doing the same thing itself. I just hoped to trick you
into working on Git. :)
-Peff
^ permalink raw reply
* Re: [PATCH 4/4] core.abbrev: raise the default abbreviation to 12 hexdigits
From: SZEDER Gábor @ 2016-09-29 12:52 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, torvalds, git
In-Reply-To: <20160929091509.2n4mdrevwxechqol@sigill.intra.peff.net>
Quoting Jeff King <peff@peff.net>:
> On Thu, Sep 29, 2016 at 04:44:00AM +0200, SZEDER Gábor wrote:
>
>> > So 12 seems reasonable, and the only downside for it (or for "13", for
>> > that matter) is a few extra bytes. I dunno, maybe people will really
>> > hate that, but I have a feeling these are mostly
>> cut-and-pasted anyway.
>>
>> I for one raise my hand in protest...
>>
>> "few extra bytes" is not the only downside, and it's not at all about
>> how many characters are copy-and-pasted. In my opinion it's much more
>> important that this change wastes 5 columns worth of valuable screen
>> real estate e.g. for 'git blame' or 'git log --oneline' in projects
>> that don't need it and certainly won't ever need it.
>
> True. The core of the issue is that we really only care about this
> minimum length when _storing_ an abbreviation, but we don't know when
> the user is just looking at it in the moment, and when they are going to
> stick it in a commit message, email, or bug tracker.
>
> In an ideal world, anybody who was about to store it would run "git
> describe" or something to come up with some canonical reference format.
> And we could just bump the default minimum there. Personally, I almost
> exclusively cite commits as the output of:
>
> git log -1 --pretty='tformat:%h (%s, %ad)' --date=short
Interesting, I have a pretty format alias that looks almost like this,
except that I carry a patch locally allowing me to say %as for short
date format :)
What I sometimes wished for is a pretty format specifier for 'git
describe --contains', which would make it convenient to cite commits
like this: v0.99~954 (Initial revision of "git", the information manager
from hell, 2005-04-07). It's better than the abbreviated object name,
because it will stay unique, assuming that the chosen tag is never
deleted, and it carries extra information for humans (the first release
containing the referenced commit), while the abbreviated object name is
completely meaningless.
The obvious drawback that makes it a non-solution for the problem at
hand is that this format can only refer to commits that are reachable
from a tag and can't be used for commits that are descendants of the
most recent tag, e.g. when fixing a bug introduced after the last
release. Oh, and the user has to fetch the tag first to be able to
make sense of such a reference.
> and I'd be fine to stick "--abbrev=12" in there for future-proofing. But
> I don't know what the kernel or other projects do.
>
> I'd also be curious to know if the patch I sent in [1] to more
> aggressively prefer commits would make this less of an issue, and people
> wouldn't care as much about using longer hashes in the first place. So
> one option is to merge that (and possibly even make it the default) and
> see if people still care in 6 months.
>
> -Peff
>
> [1]
> http://public-inbox.org/git/20160927123801.3bpdg3hap3kzzfmv@sigill.intra.peff.net/
^ permalink raw reply
* Re: [PATCH v8 00/11] Git filter protocol
From: Torsten Bögershausen @ 2016-09-29 11:57 UTC (permalink / raw)
To: Lars Schneider, Junio C Hamano
Cc: git, Jeff King, Stefan Beller, Jakub Narębski,
Martin-Louis Bright, ramsay
In-Reply-To: <C2C9761E-986F-473D-BFB7-CBEF900D9FA3@gmail.com>
On 29/09/16 12:28, Lars Schneider wrote:
>> On 28 Sep 2016, at 23:49, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> I suspect that you are preparing a reroll already, but the one that
>> is sitting in 'pu' seems to be flaky in t/t0021 and I seem to see
>> occasional failures from it.
>>
>> I didn't trace where the test goes wrong, but one easy mistake you
>> could make (I am not saying that is the reason of the failure) is to
>> assume your filter will not be called under certain condition (like
>> immediately after you checked out from the index to the working
>> tree), when the automated test goes fast enough and get you into a
>> "racy git" situation---the filter may be asked to filter the
>> contents from the working tree again to re-validate what's there is
>> still what is in the index.
> Thanks for the heads-up!
>
> This is what happens:
>
> 1) Git exits
> 2) The filter process receives EOF and prints "STOP" to the log
> 3) t0021 checks the content of the log
>
> Sometimes 3 happened before 2 which makes the test fail.
> (Example: https://travis-ci.org/git/git/jobs/162660563 )
>
> I added a this to wait until the filter process terminates:
>
> +wait_for_filter_termination () {
> + while ps | grep -v grep | grep -F "/t0021/rot13-filter.pl" >/dev/null 2>&1
> + do
> + echo "Waiting for /t0021/rot13-filter.pl to finish..."
> + sleep 1
> + done
> +}
>
> Does this look OK to you?
Do we need the ps at all ?
How about this:
+wait_for_filter_termination () {
+ while ! grep "STOP" LOGFILENAME >/dev/null
+ do
+ echo "Waiting for /t0021/rot13-filter.pl to finish..."
+ sleep 1
+ done
+}
^ permalink raw reply
* Re: [PATCH 10/10] get_short_sha1: list ambiguous objects on error
From: Kyle J. McKay @ 2016-09-29 11:46 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Linus Torvalds, Git Mailing List
In-Reply-To: <20160926120036.mqs435a36njeihq6@sigill.intra.peff.net>
On Sep 26, 2016, at 05:00, Jeff King wrote:
> $ git rev-parse b2e1
> error: short SHA1 b2e1 is ambiguous
> hint: The candidates are:
> hint: b2e1196 tag v2.8.0-rc1
> hint: b2e11d1 tree
> hint: b2e1632 commit 2007-11-14 - Merge branch 'bs/maint-commit-
> options'
> hint: b2e1759 blob
> hint: b2e18954 blob
> hint: b2e1895c blob
> fatal: ambiguous argument 'b2e1': unknown revision or path not in
> the working tree.
> Use '--' to separate paths from revisions, like this:
> 'git <command> [<revision>...] -- [<file>...]'
This hint: information is excellent. There needs to be a way to show
it on demand.
$ git rev-parse --disambiguate=b2e1
b2e11962c5e6a9c81aa712c751c83a743fd4f384
b2e11d1bb40c5f81a2f4e37b9f9a60ec7474eeab
b2e163272c01aca4aee4684f5c683ba341c1953d
b2e18954c03ff502053cb74d142faab7d2a8dacb
b2e1895ca92ec2037349d88b945ba64ebf16d62d
Not nearly so helpful, but the operation of --disambiguate cannot be
changed without breaking current scripts.
Can your excellent "hint:" output above be attached to the --
disambiguate option somehow, please. Something like this perhaps:
$ git rev-parse --disambiguate-list=b2e1
b2e1196 tag v2.8.0-rc1
b2e11d1 tree
b2e1632 commit 2007-11-14 - Merge branch 'bs/maint-commit-options'
b2e1759 blob
b2e18954 blob
b2e1895c blob
Any option name will do, --disambiguate-verbose, --disambiguate-
extended, --disambiguate-long, --disambiguate-log, --disambiguate-
help, --disambiguate-show-me-something-useful-to-humans-not-scripts ...
--Kyle
^ permalink raw reply
* Re: [PATCH/RFC] git log --oneline alternative with dates, times and initials
From: Kyle J. McKay @ 2016-09-29 11:00 UTC (permalink / raw)
To: Jeff King; +Cc: Git mailing list, Junio C Hamano
In-Reply-To: <20160929083315.vwb3aurwbyjwlkjn@sigill.intra.peff.net>
On Sep 29, 2016, at 01:33, Jeff King wrote:
> On Wed, Sep 28, 2016 at 10:34:51PM -0700, Kyle J. McKay wrote:
>
>> git log-times --graph --date-order --decorate --no-merges -n 5 v2.5.3
>>
>> === 2015-09-17 ===
>> * ee6ad5f4 12:16 jch (tag: v2.5.3) Git 2.5.3
>> === 2015-09-09 ===
>> * b9d66899 14:22 js am --skip/--abort: merge HEAD/ORIG_HEAD tree
>> into index
>> | === 2015-09-04 ===
>> | * 27ea6f85 10:46 jch (tag: v2.5.2) Git 2.5.2
>> * 74b67638 10:36 jch (tag: v2.4.9) Git 2.4.9
>> ..........
>> * ecad27cf 10:32 jch (tag: v2.3.9) Git 2.3.9
>
> I was surprised to see this as a separate script, but it is true
> that we
> cannot quite pull it off with --format. I think we are very close,
> though. With the patches below I think you can do:
>
> git log \
> --commit-header='%C(auto,bold blue)== %as ==%C(auto,reset)'
> --format='%C(auto)%h %C(auto,green)%ad %C(auto,red)%aS/%cS%C(auto)
> %d%C(auto,reset) %s' \
> --graph --no-merges --author-date-order --date=format:%H:%M
>
> and get the same (or very similar) output.
>
> [1/5]: pretty: allow formatting DATE_SHORT
> [2/5]: pretty: allow formatting names as initials
> [3/5]: graph: fix extra spaces in graph_padding_line
> [4/5]: graph: helper functions for printing commit header
> [5/5]: log: add --commit-header option
>
> Each of those commits[1] needs some minor polish, and as I'm not
> really
> that interested in fancy log output myself, I don't plan on working on
> them further. I was mostly curious just how close we were. But if
> you'd
> like to pursue it, feel free to use them as a starting point.
Those patches are missing some of the features like showing root
commits, handling two letter initials, showing the weekday, inserting
a break where needed to avoid parent-child confusion in graph output
and properly handling Duy's initials. :)
I suppose if all the objects that output a date took a '(' <strftime>
')' option that would get you part of the way -- it could replace
DATE_SHORT with DATE_STRFTIME.
Also the above example doesn't handle marks properly in graph mode.
Yes, you can add the "%m" format option but it does something odd and
the script fixes it up.
On the other hand, git-log-times started out as a script for something
else (a shell script actually) and just got embellished further and
turned into a perl script for speed.
Your patches are a good first start though but reading the --graph
code gives me headaches and I figured it would be like going down a
rabbit hole to make the code support everything the script does.
The script also has one big advantage. It works with the version of
Git everybody already has installed. :)
And nobody is ever going to want to type several lines of arcane
formatting instructions to get the output. ;_)
It would need a new option, perhaps --oneline-extended or something.
The patches are a good start but that doesn't help anyone using Git
today which is why git-log-times is submitted as a contrib script --
much like the way diff-highlight is still a contrib script and not
supported directly by Git either.
--Kyle
^ permalink raw reply
* Re: [PATCH v8 00/11] Git filter protocol
From: Lars Schneider @ 2016-09-29 10:28 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Stefan Beller, Jakub Narębski,
Martin-Louis Bright, Torsten Bögershausen, ramsay
In-Reply-To: <xmqq8tubitjs.fsf@gitster.mtv.corp.google.com>
> On 28 Sep 2016, at 23:49, Junio C Hamano <gitster@pobox.com> wrote:
>
> I suspect that you are preparing a reroll already, but the one that
> is sitting in 'pu' seems to be flaky in t/t0021 and I seem to see
> occasional failures from it.
>
> I didn't trace where the test goes wrong, but one easy mistake you
> could make (I am not saying that is the reason of the failure) is to
> assume your filter will not be called under certain condition (like
> immediately after you checked out from the index to the working
> tree), when the automated test goes fast enough and get you into a
> "racy git" situation---the filter may be asked to filter the
> contents from the working tree again to re-validate what's there is
> still what is in the index.
Thanks for the heads-up!
This is what happens:
1) Git exits
2) The filter process receives EOF and prints "STOP" to the log
3) t0021 checks the content of the log
Sometimes 3 happened before 2 which makes the test fail.
(Example: https://travis-ci.org/git/git/jobs/162660563 )
I added a this to wait until the filter process terminates:
+wait_for_filter_termination () {
+ while ps | grep -v grep | grep -F "/t0021/rot13-filter.pl" >/dev/null 2>&1
+ do
+ echo "Waiting for /t0021/rot13-filter.pl to finish..."
+ sleep 1
+ done
+}
Does this look OK to you?
- Lars
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox