Git development
 help / color / mirror / Atom feed
* [PATCH v2 0/3] count-objects improvements
From: Nguyễn Thái Ngọc Duy @ 2013-02-08  3:48 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1359982145-10792-1-git-send-email-pclouds@gmail.com>

This series:
 - updates count-objects -v documentation, describe each line in detail
 - counts garbage files in pack directory in addition to loose odb
 - shows how much disk space consumed by garbage files

Nguyễn Thái Ngọc Duy (3):
  git-count-objects.txt: describe each line in -v output
  count-objects: report garbage files in pack directory too
  count-objects: report how much disk space taken by garbage files

 Documentation/git-count-objects.txt | 22 +++++++---
 builtin/count-objects.c             | 41 ++++++++++++++-----
 sha1_file.c                         | 81 +++++++++++++++++++++++++++++++++++--
 3 files changed, 127 insertions(+), 17 deletions(-)

-- 
1.8.1.2.495.g3fdf5d5.dirty

^ permalink raw reply

* [PATCH v4] Add utf8_fprintf helper which returns correct columns
From: Jiang Xin @ 2013-02-08  2:10 UTC (permalink / raw)
  To: Junio C Hamano, Nguyễn Thái Ngọc Duy
  Cc: Git List, Jiang Xin, Nguyễn Thái Ngọc Duy
In-Reply-To: <7va9rho350.fsf@alter.siamese.dyndns.org>

Since command usages can be translated, they may not align well especially
when they are translated to CJK. A wrapper utf8_fprintf can help to return
the correct columns required.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 parse-options.c |  5 +++--
 utf8.c          | 22 ++++++++++++++++++++++
 utf8.h          |  1 +
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 67e98..a6ce9e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -3,6 +3,7 @@
 #include "cache.h"
 #include "commit.h"
 #include "color.h"
+#include "utf8.h"
 
 static int parse_options_usage(struct parse_opt_ctx_t *ctx,
 			       const char * const *usagestr,
@@ -482,7 +483,7 @@ static int usage_argh(const struct option *opts, FILE *outfile)
 			s = literal ? "[%s]" : "[<%s>]";
 	else
 		s = literal ? " %s" : " <%s>";
-	return fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
+	return utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
 }
 
 #define USAGE_OPTS_WIDTH 24
@@ -541,7 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
 		if (opts->long_name)
 			pos += fprintf(outfile, "--%s", opts->long_name);
 		if (opts->type == OPTION_NUMBER)
-			pos += fprintf(outfile, "-NUM");
+			pos += utf8_fprintf(outfile, _("-NUM"));
 
 		if ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||
 		    !(opts->flags & PARSE_OPT_NOARG))
diff --git a/utf8.c b/utf8.c
index a4ee6..05925 100644
--- a/utf8.c
+++ b/utf8.c
@@ -430,6 +430,28 @@ int same_encoding(const char *src, const char *dst)
 }
 
 /*
+ * Wrapper for fprintf and returns the total number of columns required
+ * for the printed string, assuming that the string is utf8.
+ */
+int utf8_fprintf(FILE *stream, const char *format, ...)
+{
+	struct strbuf buf = STRBUF_INIT;
+	va_list arg;
+	int columns;
+
+	va_start (arg, format);
+	strbuf_vaddf(&buf, format, arg);
+	va_end (arg);
+
+	columns = fputs(buf.buf, stream);
+	/* If no error occurs, returns columns really required with utf8_strwidth. */
+	if (0 <= columns)
+		columns = utf8_strwidth(buf.buf);
+	strbuf_release(&buf);
+	return columns;
+}
+
+/*
  * Given a buffer and its encoding, return it re-encoded
  * with iconv.  If the conversion fails, returns NULL.
  */
diff --git a/utf8.h b/utf8.h
index a2142..501b2 100644
--- a/utf8.h
+++ b/utf8.h
@@ -8,6 +8,7 @@ int utf8_strwidth(const char *string);
 int is_utf8(const char *text);
 int is_encoding_utf8(const char *name);
 int same_encoding(const char *, const char *);
+int utf8_fprintf(FILE *, const char *, ...);
 
 void strbuf_add_wrapped_text(struct strbuf *buf,
 		const char *text, int indent, int indent2, int width);
-- 
1.8.1.1.368.g917cd65

^ permalink raw reply

* Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
From: Ted Zlatanov @ 2013-02-08  1:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vtxpn4r7a.fsf@alter.siamese.dyndns.org>

On Thu, 07 Feb 2013 15:52:41 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

>> +	@echo "=> Look for any entry in the default file set"
>> +	echo "" | ./git-credential-netrc -d -v get
>> +	@echo "=> Look for github.com in the default file set"
>> +	echo "host=google.com" | ./git-credential-netrc -d -v get
>> +	@echo "=> Look for a nonexistent machine in the default file set"
>> +	echo "host=korovamilkbar" | ./git-credential-netrc -d -v get

JCH> Whose netrc is this reading?

JCH> Don't we want all of them to have "-f A" and ship "A" (rename it to
JCH> something more reasonable), so that anybody can notice when he tries
JCH> to improve it and breaks it?

I agree this Makefile is not a good test to ship out.  It was my quickie
test rig that I should have reworked before adding to the patch.  Sorry.

I see contrib/subtree/t and contrib/mw-to-git/t that I could copy.  The
test will have a few files to parse, and will be able to compare the
expected to the actual output.  Does that sound like a good plan?

Ted

^ permalink raw reply

* Re: [PATCHv2 4/5] Git.pm: add interface for git credential command
From: Michal Nazarewicz @ 2013-02-08  1:37 UTC (permalink / raw)
  To: Junio C Hamano, Matthieu Moy; +Cc: Jeff King, git
In-Reply-To: <7v7gmj66fq.fsf@alter.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 654 bytes --]

On Fri, Feb 08 2013, Junio C Hamano wrote:
> I'd actually be more worried about the error checking issue
> Peff raised during his review.  I have a feeling that "when in doubt,
> do not cause harm" is a more prudent way to go than "I do not know,
> so I'll let anything pass".

I can implement whatever checking you wish, just tell me what to do in
corner cases I've listed. ;)

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply

* [PATCH] git-mergetool: print filename when it contains %
From: Asheesh Laroia @ 2013-02-08  1:16 UTC (permalink / raw)
  To: git; +Cc: Asheesh Laroia

Before this change, if git-mergetool was invoked with regard to
files with a percent sign (%) in their names, it would print an
error. For example, if you were calling mergetool on a file called
"%2F":

    printf: %2F: invalid directive

This changes the behavior to pass "%s" to printf as its first argument
to avoid processing the filename as a format string.

Signed-off-by: Asheesh Laroia <asheesh@asheesh.org>
---
 git-mergetool.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index c50e18a..d2b9289 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -440,7 +440,7 @@ then
 fi
 
 printf "Merging:\n"
-printf "$files\n"
+printf "%s" "$files\n"
 
 IFS='
 '
-- 
1.7.10.4

^ permalink raw reply related

* [ANNOUNCE] Git v1.8.1.3
From: Junio C Hamano @ 2013-02-08  0:13 UTC (permalink / raw)
  To: git; +Cc: Linux Kernel

The latest maintenance release Git v1.8.1.3 is now available at
the usual places.

The release tarballs are found at:

    http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

29ed9047263f9835726200226451339276641779  git-1.8.1.3.tar.gz
6b1e57bde2f2b0a86532390c15bfa7b181c50db2  git-htmldocs-1.8.1.3.tar.gz
12aaa8a0428e64d194665379ab0335d786728930  git-manpages-1.8.1.3.tar.gz

Also the following public repositories all have a copy of the v1.8.1.3
tag and the maint branch that the tag points at:

  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

Git 1.8.1.3 Release Notes
=========================

Fixes since v1.8.1.2
--------------------

 * The attribute mechanism didn't allow limiting attributes to be
   applied to only a single directory itself with "path/" like the
   exclude mechanism does.  The fix for this in 1.8.1.2 had
   performance degradations.

 * Command line completion code was inadvertently made incompatible with
   older versions of bash by using a newer array notation.

 * Scripts to test bash completion was inherently flaky as it was
   affected by whatever random things the user may have on $PATH.

 * A fix was added to the build procedure to work around buggy
   versions of ccache broke the auto-generation of dependencies, which
   unfortunately is still relevant because some people use ancient
   distros.

 * We used to stuff "user@" and then append what we read from
   /etc/mailname to come up with a default e-mail ident, but a bug
   lost the "user@" part.

 * "git am" did not parse datestamp correctly from Hg generated patch,
   when it is run in a locale outside C (or en).

 * Attempt to "branch --edit-description" an existing branch, while
   being on a detached HEAD, errored out.

 * "git cherry-pick" did not replay a root commit to an unborn branch.

 * We forgot to close the file descriptor reading from "gpg" output,
   killing "git log --show-signature" on a long history.

 * "git rebase --preserve-merges" lost empty merges in recent versions
   of Git.

 * Rebasing the history of superproject with change in the submodule
   has been broken since v1.7.12.

 * A failure to push due to non-ff while on an unborn branch
   dereferenced a NULL pointer when showing an error message.

Also contains various documentation fixes.

----------------------------------------------------------------

Changes since v1.8.1.2 are as follows:

Brandon Casey (3):
      git-completion.bash: replace zsh notation that breaks bash 3.X
      git-p4.py: support Python 2.5
      git-p4.py: support Python 2.4

Dmitry V. Levin (1):
      am: invoke perl's strftime in C locale

Fraser Tweedale (1):
      push: fix segfault when HEAD points nowhere

John Keeping (1):
      git-cvsimport.txt: cvsps-2 is deprecated

Jonathan Nieder (2):
      ident: do not drop username when reading from /etc/mailname
      Makefile: explicitly set target name for autogenerated dependencies

Junio C Hamano (17):
      Which merge_file() function do you mean?
      merge-tree: lose unused "flags" from merge_list
      merge-tree: lose unused "resolve_directories"
      merge-tree: add comments to clarify what these functions are doing
      merge-tree: fix d/f conflicts
      Documentation: update "howto maintain git"
      howto/maintain: mark titles for asciidoc
      help: include <common-cmds.h> only in one file
      t9902: protect test from stray build artifacts
      howto/maintain: document "### match next" convention in jch/pu branch
      README: update stale and/or incorrect information
      INSTALL: git-p4 does not support Python 3
      git-am: record full index line in the patch used while rebasing
      apply: simplify build_fake_ancestor()
      apply: diagnose incomplete submodule object name better
      Start preparing for 1.8.1.3
      Git 1.8.1.3

Martin von Zweigbergk (2):
      tests: move test_cmp_rev to test-lib-functions
      learn to pick/revert into unborn branch

Nguyễn Thái Ngọc Duy (4):
      attr: fix off-by-one directory component length calculation
      test-lib.sh: unfilter GIT_PERF_*
      attr: avoid calling find_basename() twice per path
      branch: no detached HEAD check when editing another branch's description

Phil Hord (1):
      rebase --preserve-merges: keep all merge commits including empty ones

Ramsay Allan Jones (1):
      Makefile: Replace merge-file.h with merge-blobs.h in LIB_H

Stephen Boyd (1):
      gpg: close stderr once finished with it in verify_signed_buffer()

Torsten Bögershausen (3):
      t0050: known breakage vanished in merge (case change)
      t0050: honor CASE_INSENSITIVE_FS in add (with different case)
      t0050: Use TAB for indentation

^ permalink raw reply

* Re: [PATCH 2/2] count-objects: report garbage files in .git/objects/pack directory too
From: Duy Nguyen @ 2013-02-07 23:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzjzg802d.fsf@alter.siamese.dyndns.org>

On Fri, Feb 8, 2013 at 1:12 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> I thought about that, but we may need to do extra stat() for loose
>> garbage as well. As it is now, garbage is complained loudly, which
>> gives me enough motivation to clean up, even without looking at how
>> much disk space it uses.
>
> I wouldn't call a single line "garbage: 4" exactly *loud*.  I also
> think that this is not about *motivating* you, but about giving
> more information to the users to help them assess the health of
> their repository themselves.

That's not the only line it prints:

error: garbage found:
.git/objects/pack/pack-8074dfd2b01f494a30f02d0374baa57632d26fea.commits
error: garbage found:
.git/objects/pack/pack-834c9dccca7634c2b225db1b5050a980cb2c2de0.commits
error: garbage found: .git/objects/pack/tmp_pack_G235da
error: garbage found:
.git/objects/pack/pack-8bdf298e9252573289cd4f1e83e80c9f261882a2.commits
count: 604
size: 5576
in-pack: 172307
packs: 4
size-pack: 50421
prune-packable: 4
garbage: 4

> By the way, I wonder if we also want to notice .git/objects/garbage
> or .git/objects/ga/rbage if we are to do this?

I prefer not (for code simplicity) because we may need to catch
.git/objects/pack/info/garbage too if we do that.

>> +                     if (!has_extension(de->d_name, ".pack")) {
>> +                             report_pack_garbage(path, 0, NULL);
>> +                             continue;
>> +                     }
>
> Didn't I already say the logic should be inverted to whitelist the
> known ones?  Saying "Anything that is not '.pack' is bad" here is a
> direct opposite, I think.

I must have missed it. Will do.

>
> Add "A '.keep' file is OK" to this codeflow and see how it goes.

OK
-- 
Duy

^ permalink raw reply

* Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
From: Junio C Hamano @ 2013-02-07 23:52 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <876226p97h.fsf_-_@lifelogs.com>

Ted Zlatanov <tzz@lifelogs.com> writes:

> Add Git credential helper that can parse netrc/authinfo files.

I think this line is redundant; we already know it on the Subject: line.

> This credential helper supports multiple files, returning the first one
> that matches.  It checks file permissions and owner.  For *.gpg files,
> it will run GPG to decrypt the file.
>
> Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
> ---
> ...
> diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
> new file mode 100644
> index 0000000..18a924f
> --- /dev/null
> +++ b/contrib/credential/netrc/Makefile
> @@ -0,0 +1,12 @@
> +test_netrc:
> +	@(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring failure"
> +	@echo "=> Silent invocation... nothing should show up here with a missing file"
> +	@echo "bad data" | ./git-credential-netrc -f A get
> +	@echo "=> Back to noisy: -v and -d used below, missing file"
> +	echo "bad data" | ./git-credential-netrc -f A -d -v get
> +	@echo "=> Look for any entry in the default file set"
> +	echo "" | ./git-credential-netrc -d -v get
> +	@echo "=> Look for github.com in the default file set"
> +	echo "host=google.com" | ./git-credential-netrc -d -v get
> +	@echo "=> Look for a nonexistent machine in the default file set"
> +	echo "host=korovamilkbar" | ./git-credential-netrc -d -v get

Whose netrc is this reading?

Don't we want all of them to have "-f A" and ship "A" (rename it to
something more reasonable), so that anybody can notice when he tries
to improve it and breaks it?

^ permalink raw reply

* Re: Proposal: branch.<name>.remotepush
From: Junio C Hamano @ 2013-02-07 23:41 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Michael Schubert, Git List
In-Reply-To: <20130207233017.GD19397@google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

> The usual incantation is "git push gerrit HEAD:refs/for/master".  Is
> the code review creation push that uses a different branchname from
> the branch the integrator pulls what seems backward, or is it the need
> to specify a refname at all on the command line?
>
> I agree that a "[branch "master"] pushremote" configuration would be
> handy.  pushremote instead of remotepush to be less surprising to
> people who have already seen pushurl.

I'd actually see this as Gerrit being weird.

If it wants to quarantine a commit destined to the "master" branch,
couldn't it just let people push to "master" and then internally
update "for/master" instead?

^ permalink raw reply

* Re: [PATCHv2 4/5] Git.pm: add interface for git credential command
From: Junio C Hamano @ 2013-02-07 23:38 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Michal Nazarewicz, Jeff King, git
In-Reply-To: <vpq38x8m06f.fsf@grenoble-inp.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Michal Nazarewicz <mpn@google.com> writes:
>
>> From: Michal Nazarewicz <mina86@mina86.com>
>>
>> Add a credential() function which is an interface to the git
>> credential command.  The code is heavily based on credential_*
>> functions in <contrib/mw-to-git/git-remote-mediawiki>.
>
> I'm no perl expert, so I cannot comment much on style (there are many
> small changes compared to the mediawiki code that look like improvement
> though), but:
>
> Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>

Thanks.  I'd actually be more worried about the error checking issue
Peff raised during his review.  I have a feeling that "when in doubt,
do not cause harm" is a more prudent way to go than "I do not know,
so I'll let anything pass".

^ permalink raw reply

* Re: Proposal: branch.<name>.remotepush
From: Jonathan Nieder @ 2013-02-07 23:30 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Michael Schubert, Git List, Junio C Hamano
In-Reply-To: <CALkWK0=53riU3xKbKkyAVS8--9VoAU5P6h88MQ9-geW=H5+a-w@mail.gmail.com>

Hi Ram,

Ramkumar Ramachandra wrote:

> And yes, a regular `git push origin refs/for/master` is just retarded.

The usual incantation is "git push gerrit HEAD:refs/for/master".  Is
the code review creation push that uses a different branchname from
the branch the integrator pulls what seems backward, or is it the need
to specify a refname at all on the command line?

I agree that a "[branch "master"] pushremote" configuration would be
handy.  pushremote instead of remotepush to be less surprising to
people who have already seen pushurl.

Good luck,
Jonathan

^ permalink raw reply

* [PATCH] Use __VA_ARGS__ for all of error's arguments
From: Matt Kraai @ 2013-02-07 21:30 UTC (permalink / raw)
  To: git, Max Horn, Junio C Hamano, Jeff King; +Cc: Matt Kraai
In-Reply-To: <20130207212438.GA22253@ftbfs.org>

From: Matt Kraai <matt.kraai@amo.abbott.com>

QNX 6.3.2 uses GCC 2.95.3 by default, and GCC 2.95.3 doesn't remove the
comma if the error macro's variable argument is left out.

Instead of testing for a sufficiently recent version of GCC, make
__VA_ARGS__ match all of the arguments.

Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
---
 git-compat-util.h | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index cc2abee..2e960a9 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -305,13 +305,10 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))
 
 /*
  * Let callers be aware of the constant return value; this can help
- * gcc with -Wuninitialized analysis. We have to restrict this trick to
- * gcc, though, because of the variadic macro and the magic ## comma pasting
- * behavior. But since we're only trying to help gcc, anyway, it's OK; other
- * compilers will fall back to using the function as usual.
+ * gcc with -Wuninitialized analysis.
  */
 #if defined(__GNUC__) && ! defined(__clang__)
-#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
+#define error(...) (error(__VA_ARGS__), -1)
 #endif
 
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
-- 
1.8.1.2.546.gfc9f004

^ permalink raw reply related

* Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
From: Matt Kraai @ 2013-02-07 21:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Max Horn, Jeff King, Matt Kraai
In-Reply-To: <7vwquj6dio.fsf@alter.siamese.dyndns.org>

On Thu, Feb 07, 2013 at 01:05:19PM -0800, Junio C Hamano wrote:
> Matt Kraai <kraai@ftbfs.org> writes:
> 
> > -#if defined(__GNUC__) && ! defined(__clang__)
> > -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
> > -#endif
> > +#define error(...) (error(__VA_ARGS__), -1)
> 
> Before your change, we only define error() macro for GCC variants,
> but with your patch that no longer is the case.  Does every compiler
> that compiles Git correctly today support this style of varargs
> macros?

I don't know and I don't think it's likely I can confirm this.  I'll
submit a new patch that just changes the definition, since I don't
know of any problems other than mine with the current situation.

-- 
Matt

^ permalink raw reply

* Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
From: John Keeping @ 2013-02-07 21:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matt Kraai, git, Max Horn, Jeff King, Matt Kraai
In-Reply-To: <7vwquj6dio.fsf@alter.siamese.dyndns.org>

On Thu, Feb 07, 2013 at 01:05:19PM -0800, Junio C Hamano wrote:
> Matt Kraai <kraai@ftbfs.org> writes:
> 
> > -#if defined(__GNUC__) && ! defined(__clang__)
> > -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
> > -#endif
> > +#define error(...) (error(__VA_ARGS__), -1)
> 
> Before your change, we only define error() macro for GCC variants,
> but with your patch that no longer is the case.  Does every compiler
> that compiles Git correctly today support this style of varargs
> macros?

At the very least the "! defined(__clang__)" was only recently added
[1], although it shouldn't be needed once Clang 3.3 is released.

[1] http://article.gmane.org/gmane.comp.version-control.git/213787


John

^ permalink raw reply

* Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
From: Junio C Hamano @ 2013-02-07 21:05 UTC (permalink / raw)
  To: Matt Kraai; +Cc: git, Max Horn, Jeff King, Matt Kraai
In-Reply-To: <1360267238-21896-1-git-send-email-kraai@ftbfs.org>

Matt Kraai <kraai@ftbfs.org> writes:

> -#if defined(__GNUC__) && ! defined(__clang__)
> -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
> -#endif
> +#define error(...) (error(__VA_ARGS__), -1)

Before your change, we only define error() macro for GCC variants,
but with your patch that no longer is the case.  Does every compiler
that compiles Git correctly today support this style of varargs
macros?

^ permalink raw reply

* Re: [PATCH 0/6] Improve "git log --graph" output of merges
From: Junio C Hamano @ 2013-02-07 20:53 UTC (permalink / raw)
  To: John Keeping; +Cc: git, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>

John Keeping <john@keeping.me.uk> writes:

> This series changes a couple of places that do not currently indent
> their output when being shown with a graph.
>
> The first patch was already posted [1] and addresses the output of "git
> log --graph -c -p".  Patch 2 is an independent fix I noticed while
> working on the later patches.
>
> Patches 3-5 introduce a helper function and change existing sites using
> diff_options->output_prefix to use it, resulting in a net reduction by
> about 60 lines.  There is no functional change here.
>
> The final patch uses the helper introduced in patch 4 to make combined
> diffs should the output prefix before each line.  This affects the
> output of "git log --graph --cc [-p|--raw]".
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/215630

Overall direction looks sensible.

It is a bit scary that we are doing the the fifth one now, as
opposed to catching this repetitiveness during the initial review of
patch series that added the graph code to the diff codepath.

Thanks.

> John Keeping (6):
>   graph: output padding for merge subsequent parents
>   diff: write prefix to the correct file
>   diff.c: make constant string arguments const
>   diff: add diff_line_prefix function
>   diff.c: use diff_line_prefix() where applicable
>   combine-diff.c: teach combined diffs about line prefix
>
>  combine-diff.c |  47 +++++++++++++--------
>  diff.c         | 131 +++++++++++++++------------------------------------------
>  diff.h         |   3 ++
>  graph.c        |  10 +++++
>  4 files changed, 77 insertions(+), 114 deletions(-)

^ permalink raw reply

* [PATCH 6/6] combine-diff.c: teach combined diffs about line prefix
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>

When running "git log --graph --cc -p" the diff output for merges is not
indented by the graph structure, unlike the diffs of non-merge commits
(added in commit 7be5761 - diff.c: Output the text graph padding before
each diff line).

Fix this by teaching the combined diff code to output diff_line_prefix()
before each line.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 combine-diff.c | 47 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index bb1cc96..35c817b 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -526,7 +526,8 @@ static void show_line_to_eol(const char *line, int len, const char *reset)
 	       saw_cr_at_eol ? "\r" : "");
 }
 
-static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
+static void dump_sline(struct sline *sline, const char *line_prefix,
+		       unsigned long cnt, int num_parent,
 		       int use_color, int result_deleted)
 {
 	unsigned long mark = (1UL<<num_parent);
@@ -582,7 +583,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
 			rlines -= null_context;
 		}
 
-		fputs(c_frag, stdout);
+		printf("%s%s", line_prefix, c_frag);
 		for (i = 0; i <= num_parent; i++) putchar(combine_marker);
 		for (i = 0; i < num_parent; i++)
 			show_parent_lno(sline, lno, hunk_end, i, null_context);
@@ -614,7 +615,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
 			struct sline *sl = &sline[lno++];
 			ll = (sl->flag & no_pre_delete) ? NULL : sl->lost_head;
 			while (ll) {
-				fputs(c_old, stdout);
+				printf("%s%s", line_prefix, c_old);
 				for (j = 0; j < num_parent; j++) {
 					if (ll->parent_map & (1UL<<j))
 						putchar('-');
@@ -627,6 +628,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
 			if (cnt < lno)
 				break;
 			p_mask = 1;
+			fputs(line_prefix, stdout);
 			if (!(sl->flag & (mark-1))) {
 				/*
 				 * This sline was here to hang the
@@ -680,11 +682,13 @@ static void reuse_combine_diff(struct sline *sline, unsigned long cnt,
 static void dump_quoted_path(const char *head,
 			     const char *prefix,
 			     const char *path,
+			     const char *line_prefix,
 			     const char *c_meta, const char *c_reset)
 {
 	static struct strbuf buf = STRBUF_INIT;
 
 	strbuf_reset(&buf);
+	strbuf_addstr(&buf, line_prefix);
 	strbuf_addstr(&buf, c_meta);
 	strbuf_addstr(&buf, head);
 	quote_two_c_style(&buf, prefix, path, 0);
@@ -696,6 +700,7 @@ static void show_combined_header(struct combine_diff_path *elem,
 				 int num_parent,
 				 int dense,
 				 struct rev_info *rev,
+				 const char *line_prefix,
 				 int mode_differs,
 				 int show_file_header)
 {
@@ -714,8 +719,8 @@ static void show_combined_header(struct combine_diff_path *elem,
 		show_log(rev);
 
 	dump_quoted_path(dense ? "diff --cc " : "diff --combined ",
-			 "", elem->path, c_meta, c_reset);
-	printf("%sindex ", c_meta);
+			 "", elem->path, line_prefix, c_meta, c_reset);
+	printf("%s%sindex ", line_prefix, c_meta);
 	for (i = 0; i < num_parent; i++) {
 		abb = find_unique_abbrev(elem->parent[i].sha1,
 					 abbrev);
@@ -734,11 +739,12 @@ static void show_combined_header(struct combine_diff_path *elem,
 			    DIFF_STATUS_ADDED)
 				added = 0;
 		if (added)
-			printf("%snew file mode %06o",
-			       c_meta, elem->mode);
+			printf("%s%snew file mode %06o",
+			       line_prefix, c_meta, elem->mode);
 		else {
 			if (deleted)
-				printf("%sdeleted file ", c_meta);
+				printf("%s%sdeleted file ",
+				       line_prefix, c_meta);
 			printf("mode ");
 			for (i = 0; i < num_parent; i++) {
 				printf("%s%06o", i ? "," : "",
@@ -755,16 +761,16 @@ static void show_combined_header(struct combine_diff_path *elem,
 
 	if (added)
 		dump_quoted_path("--- ", "", "/dev/null",
-				 c_meta, c_reset);
+				 line_prefix, c_meta, c_reset);
 	else
 		dump_quoted_path("--- ", a_prefix, elem->path,
-				 c_meta, c_reset);
+				 line_prefix, c_meta, c_reset);
 	if (deleted)
 		dump_quoted_path("+++ ", "", "/dev/null",
-				 c_meta, c_reset);
+				 line_prefix, c_meta, c_reset);
 	else
 		dump_quoted_path("+++ ", b_prefix, elem->path,
-				 c_meta, c_reset);
+				 line_prefix, c_meta, c_reset);
 }
 
 static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
@@ -782,6 +788,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	struct userdiff_driver *userdiff;
 	struct userdiff_driver *textconv = NULL;
 	int is_binary;
+	const char *line_prefix = diff_line_prefix(opt);
 
 	context = opt->context;
 	userdiff = userdiff_find_by_path(elem->path);
@@ -901,7 +908,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	}
 	if (is_binary) {
 		show_combined_header(elem, num_parent, dense, rev,
-				     mode_differs, 0);
+				     line_prefix, mode_differs, 0);
 		printf("Binary files differ\n");
 		free(result);
 		return;
@@ -962,8 +969,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 
 	if (show_hunks || mode_differs || working_tree_file) {
 		show_combined_header(elem, num_parent, dense, rev,
-				     mode_differs, 1);
-		dump_sline(sline, cnt, num_parent,
+				     line_prefix, mode_differs, 1);
+		dump_sline(sline, line_prefix, cnt, num_parent,
 			   opt->use_color, result_deleted);
 	}
 	free(result);
@@ -990,6 +997,7 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
 	int i, offset;
 	const char *prefix;
 	int line_termination, inter_name_termination;
+	const char *line_prefix = diff_line_prefix(opt);
 
 	line_termination = opt->line_termination;
 	inter_name_termination = '\t';
@@ -1000,6 +1008,7 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
 		show_log(rev);
 
 	if (opt->output_format & DIFF_FORMAT_RAW) {
+		printf("%s", line_prefix);
 		offset = strlen(COLONS) - num_parent;
 		if (offset < 0)
 			offset = 0;
@@ -1040,6 +1049,7 @@ void show_combined_diff(struct combine_diff_path *p,
 		       struct rev_info *rev)
 {
 	struct diff_options *opt = &rev->diffopt;
+
 	if (!p->len)
 		return;
 	if (opt->output_format & (DIFF_FORMAT_RAW |
@@ -1150,8 +1160,10 @@ void diff_tree_combined(const unsigned char *sha1,
 
 		if (show_log_first && i == 0) {
 			show_log(rev);
+
 			if (rev->verbose_header && opt->output_format)
-				putchar(opt->line_termination);
+				printf("%s%c", diff_line_prefix(opt),
+				       opt->line_termination);
 		}
 		diff_flush(&diffopts);
 	}
@@ -1179,7 +1191,8 @@ void diff_tree_combined(const unsigned char *sha1,
 
 		if (opt->output_format & DIFF_FORMAT_PATCH) {
 			if (needsep)
-				putchar(opt->line_termination);
+				printf("%s%c", diff_line_prefix(opt),
+				       opt->line_termination);
 			for (p = paths; p; p = p->next) {
 				if (p->len)
 					show_patch_diff(p, num_parent, dense,
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 5/6] diff.c: use diff_line_prefix() where applicable
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 diff.c | 115 ++++++++++++-----------------------------------------------------
 1 file changed, 20 insertions(+), 95 deletions(-)

diff --git a/diff.c b/diff.c
index ed14d5d..f441f6c 100644
--- a/diff.c
+++ b/diff.c
@@ -402,12 +402,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
 	int nofirst;
 	FILE *file = o->file;
 
-	if (o->output_prefix) {
-		struct strbuf *msg = NULL;
-		msg = o->output_prefix(o, o->output_prefix_data);
-		assert(msg);
-		fwrite(msg->buf, msg->len, 1, file);
-	}
+	fputs(diff_line_prefix(o), file);
 
 	if (len == 0) {
 		has_trailing_newline = (first == '\n');
@@ -625,13 +620,7 @@ static void emit_rewrite_diff(const char *name_a,
 	char *data_one, *data_two;
 	size_t size_one, size_two;
 	struct emit_callback ecbdata;
-	char *line_prefix = "";
-	struct strbuf *msgbuf;
-
-	if (o && o->output_prefix) {
-		msgbuf = o->output_prefix(o, o->output_prefix_data);
-		line_prefix = msgbuf->buf;
-	}
+	const char *line_prefix = diff_line_prefix(o);
 
 	if (diff_mnemonic_prefix && DIFF_OPT_TST(o, REVERSE_DIFF)) {
 		a_prefix = o->b_prefix;
@@ -827,18 +816,14 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 	int minus_first, minus_len, plus_first, plus_len;
 	const char *minus_begin, *minus_end, *plus_begin, *plus_end;
 	struct diff_options *opt = diff_words->opt;
-	struct strbuf *msgbuf;
-	char *line_prefix = "";
+	const char *line_prefix;
 
 	if (line[0] != '@' || parse_hunk_header(line, len,
 			&minus_first, &minus_len, &plus_first, &plus_len))
 		return;
 
 	assert(opt);
-	if (opt->output_prefix) {
-		msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
-		line_prefix = msgbuf->buf;
-	}
+	line_prefix = diff_line_prefix(opt);
 
 	/* POSIX requires that first be decremented by one if len == 0... */
 	if (minus_len) {
@@ -962,14 +947,10 @@ static void diff_words_show(struct diff_words_data *diff_words)
 	struct diff_words_style *style = diff_words->style;
 
 	struct diff_options *opt = diff_words->opt;
-	struct strbuf *msgbuf;
-	char *line_prefix = "";
+	const char *line_prefix;
 
 	assert(opt);
-	if (opt->output_prefix) {
-		msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
-		line_prefix = msgbuf->buf;
-	}
+	line_prefix = diff_line_prefix(opt);
 
 	/* special case: only removal */
 	if (!diff_words->plus.text.size) {
@@ -1155,13 +1136,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 	const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
 	const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
 	struct diff_options *o = ecbdata->opt;
-	char *line_prefix = "";
-	struct strbuf *msgbuf;
-
-	if (o && o->output_prefix) {
-		msgbuf = o->output_prefix(o, o->output_prefix_data);
-		line_prefix = msgbuf->buf;
-	}
+	const char *line_prefix = diff_line_prefix(o);
 
 	if (ecbdata->header) {
 		fprintf(ecbdata->opt->file, "%s", ecbdata->header->buf);
@@ -1485,16 +1460,11 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	const char *reset, *add_c, *del_c;
 	const char *line_prefix = "";
 	int extra_shown = 0;
-	struct strbuf *msg = NULL;
 
 	if (data->nr == 0)
 		return;
 
-	if (options->output_prefix) {
-		msg = options->output_prefix(options, options->output_prefix_data);
-		line_prefix = msg->buf;
-	}
-
+	line_prefix = diff_line_prefix(options);
 	count = options->stat_count ? options->stat_count : data->nr;
 
 	reset = diff_get_color_opt(options, DIFF_RESET);
@@ -1746,12 +1716,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option
 			dels += deleted;
 		}
 	}
-	if (options->output_prefix) {
-		struct strbuf *msg = NULL;
-		msg = options->output_prefix(options,
-				options->output_prefix_data);
-		fprintf(options->file, "%s", msg->buf);
-	}
+	fprintf(options->file, "%s", diff_line_prefix(options));
 	print_stat_summary(options->file, total_files, adds, dels);
 }
 
@@ -1765,12 +1730,7 @@ static void show_numstat(struct diffstat_t *data, struct diff_options *options)
 	for (i = 0; i < data->nr; i++) {
 		struct diffstat_file *file = data->files[i];
 
-		if (options->output_prefix) {
-			struct strbuf *msg = NULL;
-			msg = options->output_prefix(options,
-					options->output_prefix_data);
-			fprintf(options->file, "%s", msg->buf);
-		}
+		fprintf(options->file, "%s", diff_line_prefix(options));
 
 		if (file->is_binary)
 			fprintf(options->file, "-\t-\t");
@@ -1812,13 +1772,7 @@ static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir,
 {
 	unsigned long this_dir = 0;
 	unsigned int sources = 0;
-	const char *line_prefix = "";
-	struct strbuf *msg = NULL;
-
-	if (opt->output_prefix) {
-		msg = opt->output_prefix(opt, opt->output_prefix_data);
-		line_prefix = msg->buf;
-	}
+	const char *line_prefix = diff_line_prefix(opt);
 
 	while (dir->nr) {
 		struct dirstat_file *f = dir->files;
@@ -2068,15 +2022,10 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
 	const char *reset = diff_get_color(data->o->use_color, DIFF_RESET);
 	const char *set = diff_get_color(data->o->use_color, DIFF_FILE_NEW);
 	char *err;
-	char *line_prefix = "";
-	struct strbuf *msgbuf;
+	const char *line_prefix;
 
 	assert(data->o);
-	if (data->o->output_prefix) {
-		msgbuf = data->o->output_prefix(data->o,
-			data->o->output_prefix_data);
-		line_prefix = msgbuf->buf;
-	}
+	line_prefix = diff_line_prefix(data->o);
 
 	if (line[0] == '+') {
 		unsigned bad;
@@ -2263,13 +2212,7 @@ static void builtin_diff(const char *name_a,
 	struct userdiff_driver *textconv_one = NULL;
 	struct userdiff_driver *textconv_two = NULL;
 	struct strbuf header = STRBUF_INIT;
-	struct strbuf *msgbuf;
-	char *line_prefix = "";
-
-	if (o->output_prefix) {
-		msgbuf = o->output_prefix(o, o->output_prefix_data);
-		line_prefix = msgbuf->buf;
-	}
+	const char *line_prefix = diff_line_prefix(o);
 
 	if (DIFF_OPT_TST(o, SUBMODULE_LOG) &&
 			(!one->mode || S_ISGITLINK(one->mode)) &&
@@ -2968,14 +2911,9 @@ static void fill_metainfo(struct strbuf *msg,
 {
 	const char *set = diff_get_color(use_color, DIFF_METAINFO);
 	const char *reset = diff_get_color(use_color, DIFF_RESET);
-	struct strbuf *msgbuf;
-	char *line_prefix = "";
+	const char *line_prefix = diff_line_prefix(o);
 
 	*must_show_header = 1;
-	if (o->output_prefix) {
-		msgbuf = o->output_prefix(o, o->output_prefix_data);
-		line_prefix = msgbuf->buf;
-	}
 	strbuf_init(msg, PATH_MAX * 2 + 300);
 	switch (p->status) {
 	case DIFF_STATUS_COPIED:
@@ -3912,12 +3850,8 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)
 {
 	int line_termination = opt->line_termination;
 	int inter_name_termination = line_termination ? '\t' : '\0';
-	if (opt->output_prefix) {
-		struct strbuf *msg = NULL;
-		msg = opt->output_prefix(opt, opt->output_prefix_data);
-		fprintf(opt->file, "%s", msg->buf);
-	}
 
+	fprintf(opt->file, "%s", diff_line_prefix(opt));
 	if (!(opt->output_format & DIFF_FORMAT_NAME_STATUS)) {
 		fprintf(opt->file, ":%06o %06o %s ", p->one->mode, p->two->mode,
 			diff_unique_abbrev(p->one->sha1, opt->abbrev));
@@ -4187,12 +4121,7 @@ static void show_rename_copy(FILE *file, const char *renamecopy, struct diff_fil
 static void diff_summary(struct diff_options *opt, struct diff_filepair *p)
 {
 	FILE *file = opt->file;
-	char *line_prefix = "";
-
-	if (opt->output_prefix) {
-		struct strbuf *buf = opt->output_prefix(opt, opt->output_prefix_data);
-		line_prefix = buf->buf;
-	}
+	const char *line_prefix = diff_line_prefix(opt);
 
 	switch(p->status) {
 	case DIFF_STATUS_DELETED:
@@ -4493,13 +4422,9 @@ void diff_flush(struct diff_options *options)
 
 	if (output_format & DIFF_FORMAT_PATCH) {
 		if (separator) {
-			if (options->output_prefix) {
-				struct strbuf *msg = NULL;
-				msg = options->output_prefix(options,
-					options->output_prefix_data);
-				fwrite(msg->buf, msg->len, 1, options->file);
-			}
-			putc(options->line_termination, options->file);
+			fprintf(options->file, "%s%c",
+				diff_line_prefix(options),
+				options->line_termination);
 			if (options->stat_sep) {
 				/* attach patch instead of inline */
 				fputs(options->stat_sep, options->file);
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 0/6] Improve "git log --graph" output of merges
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King

This series changes a couple of places that do not currently indent
their output when being shown with a graph.

The first patch was already posted [1] and addresses the output of "git
log --graph -c -p".  Patch 2 is an independent fix I noticed while
working on the later patches.

Patches 3-5 introduce a helper function and change existing sites using
diff_options->output_prefix to use it, resulting in a net reduction by
about 60 lines.  There is no functional change here.

The final patch uses the helper introduced in patch 4 to make combined
diffs should the output prefix before each line.  This affects the
output of "git log --graph --cc [-p|--raw]".

[1] http://article.gmane.org/gmane.comp.version-control.git/215630

John Keeping (6):
  graph: output padding for merge subsequent parents
  diff: write prefix to the correct file
  diff.c: make constant string arguments const
  diff: add diff_line_prefix function
  diff.c: use diff_line_prefix() where applicable
  combine-diff.c: teach combined diffs about line prefix

 combine-diff.c |  47 +++++++++++++--------
 diff.c         | 131 +++++++++++++++------------------------------------------
 diff.h         |   3 ++
 graph.c        |  10 +++++
 4 files changed, 77 insertions(+), 114 deletions(-)

-- 
1.8.1.2

^ permalink raw reply

* [PATCH 4/6] diff: add diff_line_prefix function
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>

This is a helper function to call the diff output_prefix function and
return its value as a C string, allowing us to greatly simplify
everywhere that needs to get the output prefix.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 diff.c | 10 ++++++++++
 diff.h |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/diff.c b/diff.c
index 73ae02d..ed14d5d 100644
--- a/diff.c
+++ b/diff.c
@@ -1105,6 +1105,16 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix)
 	return "";
 }
 
+const char *diff_line_prefix(struct diff_options *opt)
+{
+	struct strbuf *msgbuf;
+	if (!opt->output_prefix)
+		return "";
+
+	msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
+	return msgbuf->buf;
+}
+
 static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, unsigned long len)
 {
 	const char *cp;
diff --git a/diff.h b/diff.h
index a47bae4..76830e2 100644
--- a/diff.h
+++ b/diff.h
@@ -174,6 +174,9 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix);
 	diff_get_color((o)->use_color, ix)
 
 
+const char *diff_line_prefix(struct diff_options *);
+
+
 extern const char mime_boundary_leader[];
 
 extern void diff_tree_setup_paths(const char **paths, struct diff_options *);
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 1/6] graph: output padding for merge subsequent parents
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>

When showing merges in git-log, the same commit is shown once for each
parent.  Combined with "--graph" this results in graph_show_commit()
being called once for each parent without graph_update() being called.

Currently graph_show_commit() does not print anything on subsequent
invocations for the same commit (this was changed by commit 656197a -
"graph.c: infinite loop in git whatchanged --graph -m" from the previous
behaviour of looping infinitely).

Change this so that if the graph code believes it has already shown the
commit it prints a single padding line.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 graph.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/graph.c b/graph.c
index 391a712..2a3fc5c 100644
--- a/graph.c
+++ b/graph.c
@@ -1227,6 +1227,16 @@ void graph_show_commit(struct git_graph *graph)
 	if (!graph)
 		return;
 
+	/*
+	 * When showing a diff of a merge against each of its parents, we
+	 * are called once for each parent without graph_update having been
+	 * called.  In this case, simply output a single padding line.
+	 */
+	if (graph_is_commit_finished(graph)) {
+		graph_show_padding(graph);
+		shown_commit_line = 1;
+	}
+
 	while (!shown_commit_line && !graph_is_commit_finished(graph)) {
 		shown_commit_line = graph_next_line(graph, &msgbuf);
 		fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 2/6] diff: write prefix to the correct file
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>

Write the prefix for an output line to the same file as the actual
content.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 348f71b..bf95235 100644
--- a/diff.c
+++ b/diff.c
@@ -4485,7 +4485,7 @@ void diff_flush(struct diff_options *options)
 				struct strbuf *msg = NULL;
 				msg = options->output_prefix(options,
 					options->output_prefix_data);
-				fwrite(msg->buf, msg->len, 1, stdout);
+				fwrite(msg->buf, msg->len, 1, options->file);
 			}
 			putc(options->line_termination, options->file);
 			if (options->stat_sep) {
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 3/6] diff.c: make constant string arguments const
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 diff.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index bf95235..73ae02d 100644
--- a/diff.c
+++ b/diff.c
@@ -2123,7 +2123,8 @@ static unsigned char *deflate_it(char *data,
 	return deflated;
 }
 
-static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two, char *prefix)
+static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two,
+				  const char *prefix)
 {
 	void *cp;
 	void *delta;
@@ -2184,7 +2185,8 @@ static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two, char
 	free(data);
 }
 
-static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two, char *prefix)
+static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two,
+			     const char *prefix)
 {
 	fprintf(file, "%sGIT binary patch\n", prefix);
 	emit_binary_diff_body(file, one, two, prefix);
-- 
1.8.1.2

^ permalink raw reply related

* Re: Proposal: branch.<name>.remotepush
From: Ramkumar Ramachandra @ 2013-02-07 20:14 UTC (permalink / raw)
  To: Michael Schubert; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0=_0hZP_SjMCjooUAr+MrRoXCCzdQF8y9RhW1g7ShsC7w@mail.gmail.com>

Ramkumar Ramachandra wrote:
> Ramkumar Ramachandra wrote:
>> And yes, a regular `git push origin refs/for/master` is just retarded.
>
> Actually a git config remote.origin.push refs/heads/*:refs/for/* makes
> more sense here.

Sorry about all that confusion.  The first line should be `git push
origin master:refs/for/master`, but a rule like refs/head/*:refs/for/*
is insufficient: what if I want refs/head/*:refs/heads/* for one set
of branches (private ones that I don't send for review), and
refs/heads/*:refs/for/* for another set (which I send for review)?
That certainly won't play well will the existing remote.origin.push;
it'd have to behave as an override.

^ permalink raw reply

* [PATCH] Use __VA_ARGS__ for all of error's arguments
From: Matt Kraai @ 2013-02-07 20:00 UTC (permalink / raw)
  To: git, Max Horn, Jeff King, Junio C Hamano; +Cc: Matt Kraai

From: Matt Kraai <matt.kraai@amo.abbott.com>

QNX 6.3.2 uses GCC 2.95.3 by default, and GCC 2.95.3 doesn't remove the
comma if the error macro's variable argument is left out.

Instead of testing for a sufficiently recent version of GCC, make
__VA_ARGS__ match all of the arguments.  Since this should work on any
C99-compliant compiler, also remove the tests around the macro definition.

Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
---
 git-compat-util.h | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index cc2abee..df1681f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -305,14 +305,9 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))
 
 /*
  * Let callers be aware of the constant return value; this can help
- * gcc with -Wuninitialized analysis. We have to restrict this trick to
- * gcc, though, because of the variadic macro and the magic ## comma pasting
- * behavior. But since we're only trying to help gcc, anyway, it's OK; other
- * compilers will fall back to using the function as usual.
+ * gcc with -Wuninitialized analysis.
  */
-#if defined(__GNUC__) && ! defined(__clang__)
-#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
-#endif
+#define error(...) (error(__VA_ARGS__), -1)
 
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
 extern void set_error_routine(void (*routine)(const char *err, va_list params));
-- 
1.8.1.GIT

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox