Git development
 help / color / mirror / Atom feed
* [PATCH 6/6] i18n: stash: mark messages for translation
From: Vasco Almeida @ 2016-09-19 13:08 UTC (permalink / raw)
  To: git
  Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
	Jean-Noël AVILA
In-Reply-To: <1474290501-2743-1-git-send-email-vascomalmeida@sapo.pt>

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 git-stash.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 826af18..90d63f2 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -100,7 +100,7 @@ create_stash () {
 				u_tree=$(git write-tree) &&
 				printf 'untracked files on %s\n' "$msg" | git commit-tree $u_tree  &&
 				rm -f "$TMPindex"
-		) ) || die "Cannot save the untracked files"
+		) ) || die "$(gettext "Cannot save the untracked files")"
 
 		untracked_commit_option="-p $u_commit";
 	else
@@ -248,7 +248,7 @@ save_stash () {
 
 	if test -n "$patch_mode" && test -n "$untracked"
 	then
-	    die "Can't use --patch and --include-untracked or --all at the same time"
+		die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
 	fi
 
 	stash_msg="$*"
@@ -494,7 +494,7 @@ apply_stash () {
 		GIT_INDEX_FILE="$TMPindex" git-read-tree "$u_tree" &&
 		GIT_INDEX_FILE="$TMPindex" git checkout-index --all &&
 		rm -f "$TMPindex" ||
-		die 'Could not restore untracked files from stash'
+		die "$(gettext "Could not restore untracked files from stash")"
 	fi
 
 	eval "
-- 
2.7.4


^ permalink raw reply related

* [PATCH 5/6] i18n: notes-merge: mark die messages for translation
From: Vasco Almeida @ 2016-09-19 13:08 UTC (permalink / raw)
  To: git
  Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
	Jean-Noël AVILA
In-Reply-To: <1474290501-2743-1-git-send-email-vascomalmeida@sapo.pt>

Update test to reflect changes.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 notes-merge.c                         | 8 ++++----
 t/t3310-notes-merge-manual-resolve.sh | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/notes-merge.c b/notes-merge.c
index 97fc42f..3bbeb86 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -269,15 +269,15 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)
 		if (file_exists(git_path(NOTES_MERGE_WORKTREE)) &&
 		    !is_empty_dir(git_path(NOTES_MERGE_WORKTREE))) {
 			if (advice_resolve_conflict)
-				die("You have not concluded your previous "
+				die(_("You have not concluded your previous "
 				    "notes merge (%s exists).\nPlease, use "
 				    "'git notes merge --commit' or 'git notes "
 				    "merge --abort' to commit/abort the "
 				    "previous merge before you start a new "
-				    "notes merge.", git_path("NOTES_MERGE_*"));
+				    "notes merge."), git_path("NOTES_MERGE_*"));
 			else
-				die("You have not concluded your notes merge "
-				    "(%s exists).", git_path("NOTES_MERGE_*"));
+				die(_("You have not concluded your notes merge "
+				    "(%s exists)."), git_path("NOTES_MERGE_*"));
 		}
 
 		if (safe_create_leading_directories_const(git_path(
diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index 6967436..baef2d6 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -225,7 +225,7 @@ test_expect_success 'cannot do merge w/conflicts when previous merge is unfinish
 	test -d .git/NOTES_MERGE_WORKTREE &&
 	test_must_fail git notes merge z >output 2>&1 &&
 	# Output should indicate what is wrong
-	grep -q "\\.git/NOTES_MERGE_\\* exists" output
+	test_i18ngrep -q "\\.git/NOTES_MERGE_\\* exists" output
 '
 
 # Setup non-conflicting merge between x and new notes ref w
-- 
2.7.4


^ permalink raw reply related

* [PATCH 3/6] i18n: diff: mark die errors for translation
From: Vasco Almeida @ 2016-09-19 13:08 UTC (permalink / raw)
  To: git
  Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
	Jean-Noël AVILA
In-Reply-To: <1474290501-2743-1-git-send-email-vascomalmeida@sapo.pt>

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 diff.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/diff.c b/diff.c
index b4310f8..d82ad79 100644
--- a/diff.c
+++ b/diff.c
@@ -3325,7 +3325,7 @@ void diff_setup_done(struct diff_options *options)
 	if (options->output_format & DIFF_FORMAT_NO_OUTPUT)
 		count++;
 	if (count > 1)
-		die("--name-only, --name-status, --check and -s are mutually exclusive");
+		die(_("--name-only, --name-status, --check and -s are mutually exclusive"));
 
 	/*
 	 * Most of the time we can say "there are changes"
@@ -3521,7 +3521,7 @@ static int stat_opt(struct diff_options *options, const char **av)
 			if (*arg == '=')
 				width = strtoul(arg + 1, &end, 10);
 			else if (!*arg && !av[1])
-				die("Option '--stat-width' requires a value");
+				die(_("Option '--stat-width' requires a value"));
 			else if (!*arg) {
 				width = strtoul(av[1], &end, 10);
 				argcount = 2;
@@ -3530,7 +3530,7 @@ static int stat_opt(struct diff_options *options, const char **av)
 			if (*arg == '=')
 				name_width = strtoul(arg + 1, &end, 10);
 			else if (!*arg && !av[1])
-				die("Option '--stat-name-width' requires a value");
+				die(_("Option '--stat-name-width' requires a value"));
 			else if (!*arg) {
 				name_width = strtoul(av[1], &end, 10);
 				argcount = 2;
@@ -3539,7 +3539,7 @@ static int stat_opt(struct diff_options *options, const char **av)
 			if (*arg == '=')
 				graph_width = strtoul(arg + 1, &end, 10);
 			else if (!*arg && !av[1])
-				die("Option '--stat-graph-width' requires a value");
+				die(_("Option '--stat-graph-width' requires a value"));
 			else if (!*arg) {
 				graph_width = strtoul(av[1], &end, 10);
 				argcount = 2;
@@ -3548,7 +3548,7 @@ static int stat_opt(struct diff_options *options, const char **av)
 			if (*arg == '=')
 				count = strtoul(arg + 1, &end, 10);
 			else if (!*arg && !av[1])
-				die("Option '--stat-count' requires a value");
+				die(_("Option '--stat-count' requires a value"));
 			else if (!*arg) {
 				count = strtoul(av[1], &end, 10);
 				argcount = 2;
-- 
2.7.4


^ permalink raw reply related

* [PATCH 4/6] i18n: ident: mark hint for translation
From: Vasco Almeida @ 2016-09-19 13:08 UTC (permalink / raw)
  To: git
  Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
	Jean-Noël AVILA
In-Reply-To: <1474290501-2743-1-git-send-email-vascomalmeida@sapo.pt>

Mark env_hint for translation.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 ident.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/ident.c b/ident.c
index e20a772..92c3cca 100644
--- a/ident.c
+++ b/ident.c
@@ -331,17 +331,17 @@ int split_ident_line(struct ident_split *split, const char *line, int len)
 }
 
 static const char *env_hint =
-"\n"
-"*** Please tell me who you are.\n"
-"\n"
-"Run\n"
-"\n"
-"  git config --global user.email \"you@example.com\"\n"
-"  git config --global user.name \"Your Name\"\n"
-"\n"
-"to set your account\'s default identity.\n"
-"Omit --global to set the identity only in this repository.\n"
-"\n";
+N_("\n"
+   "*** Please tell me who you are.\n"
+   "\n"
+   "Run\n"
+   "\n"
+   "  git config --global user.email \"you@example.com\"\n"
+   "  git config --global user.name \"Your Name\"\n"
+   "\n"
+   "to set your account\'s default identity.\n"
+   "Omit --global to set the identity only in this repository.\n"
+   "\n");
 
 const char *fmt_ident(const char *name, const char *email,
 		      const char *date_str, int flag)
@@ -356,13 +356,13 @@ const char *fmt_ident(const char *name, const char *email,
 		if (!name) {
 			if (strict && ident_use_config_only
 			    && !(ident_config_given & IDENT_NAME_GIVEN)) {
-				fputs(env_hint, stderr);
+				fputs(_(env_hint), stderr);
 				die("no name was given and auto-detection is disabled");
 			}
 			name = ident_default_name();
 			using_default = 1;
 			if (strict && default_name_is_bogus) {
-				fputs(env_hint, stderr);
+				fputs(_(env_hint), stderr);
 				die("unable to auto-detect name (got '%s')", name);
 			}
 		}
@@ -370,7 +370,7 @@ const char *fmt_ident(const char *name, const char *email,
 			struct passwd *pw;
 			if (strict) {
 				if (using_default)
-					fputs(env_hint, stderr);
+					fputs(_(env_hint), stderr);
 				die("empty ident name (for <%s>) not allowed", email);
 			}
 			pw = xgetpwuid_self(NULL);
@@ -381,12 +381,12 @@ const char *fmt_ident(const char *name, const char *email,
 	if (!email) {
 		if (strict && ident_use_config_only
 		    && !(ident_config_given & IDENT_MAIL_GIVEN)) {
-			fputs(env_hint, stderr);
+			fputs(_(env_hint), stderr);
 			die("no email was given and auto-detection is disabled");
 		}
 		email = ident_default_email();
 		if (strict && default_email_is_bogus) {
-			fputs(env_hint, stderr);
+			fputs(_(env_hint), stderr);
 			die("unable to auto-detect email address (got '%s')", email);
 		}
 	}
-- 
2.7.4


^ permalink raw reply related

* [PATCH 2/6] i18n: connect: mark die messages for translation
From: Vasco Almeida @ 2016-09-19 13:08 UTC (permalink / raw)
  To: git
  Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
	Jean-Noël AVILA
In-Reply-To: <1474290501-2743-1-git-send-email-vascomalmeida@sapo.pt>

Mark messages passed to die() in die_initial_contact().

Update test to reflect changes.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 connect.c            | 8 ++++----
 t/t5512-ls-remote.sh | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/connect.c b/connect.c
index 722dc3f..06bff0b 100644
--- a/connect.c
+++ b/connect.c
@@ -46,11 +46,11 @@ int check_ref_type(const struct ref *ref, int flags)
 static void die_initial_contact(int got_at_least_one_head)
 {
 	if (got_at_least_one_head)
-		die("The remote end hung up upon initial contact");
+		die(_("The remote end hung up upon initial contact"));
 	else
-		die("Could not read from remote repository.\n\n"
-		    "Please make sure you have the correct access rights\n"
-		    "and the repository exists.");
+		die(_("Could not read from remote repository.\n\n"
+		      "Please make sure you have the correct access rights\n"
+		      "and the repository exists."));
 }
 
 static void parse_one_symref_info(struct string_list *symref, const char *val, int len)
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 819b9dd..c23434b 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -99,7 +99,7 @@ test_expect_success 'confuses pattern as remote when no remote specified' '
 	# We could just as easily have used "master"; the "*" emphasizes its
 	# role as a pattern.
 	test_must_fail git ls-remote refs*master >actual 2>&1 &&
-	test_cmp exp actual
+	test_i18ncmp exp actual
 '
 
 test_expect_success 'die with non-2 for wrong repository even with --exit-code' '
-- 
2.7.4


^ permalink raw reply related

* [PATCH 1/6] i18n: commit: mark message for translation
From: Vasco Almeida @ 2016-09-19 13:08 UTC (permalink / raw)
  To: git
  Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
	Jean-Noël AVILA

Mark message commit_utf8_warn for translation.

Update tests to reflect changes.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 commit.c               | 8 ++++----
 t/t3900-i18n-commit.sh | 8 ++++----
 t/t3901-i18n-patch.sh  | 2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/commit.c b/commit.c
index ba6dee3..8eb1707 100644
--- a/commit.c
+++ b/commit.c
@@ -1511,9 +1511,9 @@ static int verify_utf8(struct strbuf *buf)
 }
 
 static const char commit_utf8_warn[] =
-"Warning: commit message did not conform to UTF-8.\n"
-"You may want to amend it after fixing the message, or set the config\n"
-"variable i18n.commitencoding to the encoding your project uses.\n";
+N_("Warning: commit message did not conform to UTF-8.\n"
+   "You may want to amend it after fixing the message, or set the config\n"
+   "variable i18n.commitencoding to the encoding your project uses.\n");
 
 int commit_tree_extended(const char *msg, size_t msg_len,
 			 const unsigned char *tree,
@@ -1566,7 +1566,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 
 	/* And check the encoding */
 	if (encoding_is_utf8 && !verify_utf8(&buffer))
-		fprintf(stderr, commit_utf8_warn);
+		fprintf(stderr, _(commit_utf8_warn));
 
 	if (sign_commit && do_sign_commit(&buffer, sign_commit))
 		return -1;
diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
index 4bf1dbe..3b94283 100755
--- a/t/t3900-i18n-commit.sh
+++ b/t/t3900-i18n-commit.sh
@@ -45,7 +45,7 @@ test_expect_success 'UTF-8 invalid characters refused' '
 	printf "Commit message\n\nInvalid surrogate:\355\240\200\n" \
 		>"$HOME/invalid" &&
 	git commit -a -F "$HOME/invalid" 2>"$HOME"/stderr &&
-	grep "did not conform" "$HOME"/stderr
+	test_i18ngrep "did not conform" "$HOME"/stderr
 '
 
 test_expect_success 'UTF-8 overlong sequences rejected' '
@@ -55,7 +55,7 @@ test_expect_success 'UTF-8 overlong sequences rejected' '
 	printf "\340\202\251ommit message\n\nThis is not a space:\300\240\n" \
 		>"$HOME/invalid" &&
 	git commit -a -F "$HOME/invalid" 2>"$HOME"/stderr &&
-	grep "did not conform" "$HOME"/stderr
+	test_i18ngrep "did not conform" "$HOME"/stderr
 '
 
 test_expect_success 'UTF-8 non-characters refused' '
@@ -64,7 +64,7 @@ test_expect_success 'UTF-8 non-characters refused' '
 	printf "Commit message\n\nNon-character:\364\217\277\276\n" \
 		>"$HOME/invalid" &&
 	git commit -a -F "$HOME/invalid" 2>"$HOME"/stderr &&
-	grep "did not conform" "$HOME"/stderr
+	test_i18ngrep "did not conform" "$HOME"/stderr
 '
 
 test_expect_success 'UTF-8 non-characters refused' '
@@ -73,7 +73,7 @@ test_expect_success 'UTF-8 non-characters refused' '
 	printf "Commit message\n\nNon-character:\357\267\220\n" \
 		>"$HOME/invalid" &&
 	git commit -a -F "$HOME/invalid" 2>"$HOME"/stderr &&
-	grep "did not conform" "$HOME"/stderr
+	test_i18ngrep "did not conform" "$HOME"/stderr
 '
 
 for H in ISO8859-1 eucJP ISO-2022-JP
diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
index 509084e..f663d56 100755
--- a/t/t3901-i18n-patch.sh
+++ b/t/t3901-i18n-patch.sh
@@ -295,7 +295,7 @@ test_expect_success 'am --no-utf8 (U/L)' '
 
 	# commit-tree will warn that the commit message does not contain valid UTF-8
 	# as mailinfo did not convert it
-	grep "did not conform" err &&
+	test_i18ngrep "did not conform" err &&
 
 	check_encoding 2
 '
-- 
2.7.4


^ permalink raw reply related

* Re: Two bugs in --pretty with %C(auto)
From: Duy Nguyen @ 2016-09-19 12:59 UTC (permalink / raw)
  To: René Scharfe; +Cc: Anatoly Borodin, Git Mailing List, Junio C Hamano
In-Reply-To: <db20ae0c-9c33-1e65-b201-1b6a9ed11340@web.de>

On Sun, Sep 18, 2016 at 1:25 AM, René Scharfe <l.s.r@web.de> wrote:
> Am 17.09.2016 um 14:51 schrieb Anatoly Borodin:
>> Hi All!
>>
>> First bug:
>>
>>       git log -3 --pretty='%C(cyan)%C(auto)%h%C(auto)%d %s'
>>
>> prints %h with the default color (normal yellow), but
>>
>>       git log -3 --pretty='%C(bold cyan)%C(auto)%h%C(auto)%d %s'
>>
>> shows %h with bold yellow, as if only the color was reset, but not
>> the attributes (blink, ul, reverse also work this way). %d and %s are
>> printed with the right color both times.
>>
>> Second bug, maybe related to the first one:
>>
>>       git log -3 --pretty='%C(bold cyan)%h%C(auto)%d %s %an %h %h %s'
>>
>> The first line looks as expected. Well, almost: the '(' of %d is bold
>> yellow.
>>
>> The second line looks like this:
>>
>> * %h, %s, %an with bold cyan;
>> * %h with bold yellow;
>> * %h with normal yellow and %s with normal white (default colors).
>>
>> PS git version 2.9.2
>
> Well, in both cases you could add %Creset before %C(auto) to get what
> you want.
>
> I'm not sure how just how automatic %C(auto) is supposed to be, but you
> expected it do emit the reset for you, right?  Sounds reasonable to me.
> The following patch implements that behavior.
>
> Duy, what do you think?

Even though letting some attributes before %C(auto) through sounds
interesting, I'd say it's a bit unpredictable, especially when the
main usage of %C(auto) is %d which could use plenty of colors. So yes,
your changes look good.
-- 
Duy

^ permalink raw reply

* clarification of `rev-list --no-walk ^<rev>`?
From: Philip Oakley @ 2016-09-19 10:56 UTC (permalink / raw)
  To: Git List

A question came up on the Git user list regarding cherry-pick that got me
reading the manual (again), in this case regarding --no-walk ranges.

Essentially my question is: If --no-walk is given to rev-list (e.g. via
charry-pick), and the user includes a caret prefixed rev, when does that
range definition take effect on the command line, especially in light of
the --do-walk option?

In rev-list(1) there are only 8 references to  'range', with only
the --no-walk option saying "This has no effect if a range is specified."
but leaving open the decision as to what does (and does not) comprises the
specification of a range on the cli.

The two and three dot notations are fairly obvious ranges from
gitrevisions(7) as they are complete strings, while the caret prefix is an 
implied range (it needs additional parameters to complete the range, and 
there-in lies the issue).

It can be read that

$ git cherry-pick maint next

would pick two single commits, while

$ git cherry-pick maint next ^master

could implicitly be read as

$ git cherry-pick maint next --do-walk ^master

because the ^ caret starts the range that cancels the --no-walk.

Clearly that's not what is intended, which is

$ git cherry-pick --do-walk maint next ^master

but it is open to interpretation as to where in the command line the caret
range prefix's --do-walk (to countermand the --no-walk) should applied.

If the user did want just the single commit at the tip of maint, and then
the range master..next, what would be their command line, and also, how
would the man page warn against false expectations?

--

Philip






^ permalink raw reply

* Re: [PATCH] mailinfo: unescape quoted-pair in header fields
From: Kevin Daudt @ 2016-09-19 10:51 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Swift Geek, Junio C Hamano
In-Reply-To: <20160916222206.jz2d4gpaxxccia5p@sigill.intra.peff.net>

Thanks for the review

On Fri, Sep 16, 2016 at 03:22:06PM -0700, Jeff King wrote:
> On Fri, Sep 16, 2016 at 11:02:04PM +0200, Kevin Daudt wrote:
> 
> >  mailinfo.c                 | 54 ++++++++++++++++++++++++++++++++++++++++++++++
> >  t/t5100-mailinfo.sh        |  6 ++++++
> >  t/t5100/quoted-pair.expect |  5 +++++
> >  t/t5100/quoted-pair.in     |  9 ++++++++
> >  4 files changed, 74 insertions(+)
> >  create mode 100644 t/t5100/quoted-pair.expect
> >  create mode 100644 t/t5100/quoted-pair.in
> > 
> > diff --git a/mailinfo.c b/mailinfo.c
> > index e19abe3..04036f3 100644
> > --- a/mailinfo.c
> > +++ b/mailinfo.c
> > @@ -54,15 +54,69 @@ static void parse_bogus_from(struct mailinfo *mi, const struct strbuf *line)
> >  	get_sane_name(&mi->name, &mi->name, &mi->email);
> >  }
> >  
> > +static int unquote_quoted_string(struct strbuf *line)
> > +{
> > +	struct strbuf outbuf;
> > +	const char *in = line->buf;
> > +	int c, take_next_literally = 0;
> > +	int found_error = 0;
> > +	char escape_context=0;
> 
> Style: whitespace around "=".
> 
> I had to wonder why we needed both escape_context and
> take_next_literally; shouldn't we just need a single state bit. But
> escape_context is not "escape the next character", it is "we are
> currently in a mode where we should be escaping".
> 
> Could we give it a more descriptive name? I guess it is more than just
> "we are in a mode", but rather "here is the character that will end the
> escaped mode". Maybe a comment would be more appropriate.
> 

Yes, your analysis is right, we need to know what character would end
the 'escape context'. I'll add a comment.

> > +	while ((c = *in++) != 0) {
> > +		if (take_next_literally) {
> > +			take_next_literally = 0;
> > +		} else {
> 
> OK, so that means the previous one was backslash-quoted, and we don't do
> any other cleverness. Good.
> 
> > +			switch (c) {
> > +			case '"':
> > +				if (!escape_context)
> > +					escape_context = '"';
> > +				else if (escape_context == '"')
> > +					escape_context = 0;
> > +				continue;
> 
> And here we open or close the quoted portion, depending. Makes sense.
> 
> > +			case '\\':
> > +				if (escape_context) {
> > +					take_next_literally = 1;
> > +					continue;
> > +				}
> > +				break;
> 
> I didn't look in the RFC. Is:
> 
>   From: my \"name\" <foo@example.com>
> 
> really the same as:
> 
>   From: "my \\\"name\\\"" <foo@example.com>
> 
> ? That seems weird, but I think it may be that the former is simply
> bogus (you are not supposed to use backslashes outside of the quoted
> section at all).

Correct, the quoted-pair (escape sequence) can only occur in a quoted
string or a comment. Even more so, the display name *needs* to be quoted
when consisting of more then one word according to the RFC.

> 
> > +			case '(':
> > +				if (!escape_context)
> > +					escape_context = '(';
> > +				else if (escape_context == '(')
> > +					found_error = 1;
> > +				break;
> 
> Hmm. Is:
> 
>   From: Name (Comment with (another comment))
> 
> really disallowed? RFC2822 seems to say that "comment" can contain
> "ccontent", which can itself be a comment.

Yes, you are right, it is allowed, I was just looking at the ctext when
adding this, but failed to see that comments can be nested at that time.

> 
> This is obviously getting pretty silly, but if we are going to follow
> the RFC, I think you actually have to do a recursive parse, and keep
> track of an arbitrary depth of context.
> 
> I dunno. This method probably covers most cases in practice, and it's
> easy to reason about.

The problem is, how do you differentiate between nested comments, and
escaped braces within a comment after one run?
> 
> > +			case ')':
> > +				if (escape_context == '(')
> > +					escape_context = 0;
> > +				break;
> > +			}
> > +		}
> > +
> > +		strbuf_addch(&outbuf, c);
> > +	}
> > +
> > +	strbuf_reset(line);
> > +	strbuf_addbuf(line, &outbuf);
> > +	strbuf_release(&outbuf);
> 
> I think you can use strbuf_swap() here to avoid copying the line an
> extra time, like:
> 
>   strbuf_swap(line, &outbuf);
>   strbuf_release(&outbuf);
> 
> Another option would be to just:
> 
>   in = strbuf_detach(&line);
> 
> at the beginning, and then output back into "line".
> 

Thanks, I just looked at what other functions were doing, but this is
much better indeed.

> > +	return found_error;
> 
> What happens when we get here and take_next_literally is set? I.e., a
> backslash at the end of the string. We'll silently print nothing, which
> seems reasonable to me (the other option is to print a literal
> backslash).
> 
> Ditto, what if escape_context is non-zero? We're in the middle of an
> unterminated quoted string (or comment).
> 
> I'm fine with silently continuing, but it seems weird that we notice
> embedded comments (and return an error), but not these other conditions.
> 

I agree. I'm thinking it's better to just be lenient in this method. If
a quote wasn't properly closed, there would be no e-mail adress for
example. I think it would do little harm, and I'd remove the checking
for the opening brace too.

> >  static void handle_from(struct mailinfo *mi, const struct strbuf *from)
> >  {
> >  	char *at;
> >  	size_t el;
> >  	struct strbuf f;
> >  
> > +
> >  	strbuf_init(&f, from->len);
> >  	strbuf_addbuf(&f, from);
> 
> Funny extra line?

Ugh

> 
> > +test_expect_success 'mailinfo unescapes rfc2822 quoted-string' '
> > +    mkdir quoted-pair &&
> > +    git mailinfo /dev/null /dev/null <"$TEST_DIRECTORY"/t5100/quoted-pair.in >quoted-pair/info &&
> > +    test_cmp "$TEST_DIRECTORY"/t5100/quoted-pair.expect quoted-pair/info
> > +'
> 
> We usually break long lines with backslash-escapes. Like:
> 
>   git mailinfo /dev/null /dev/null \
> 	<"$TEST_DIRECTORY"/t5100/quoted-pair.in \
> 	>quoted-pair/info
> 
> I'd also wonder if things might be made much more readable by putting
> "$TEST_DIRECTORY/t5100" into a shorter variable like $data or something.
> That would be best done as a preparatory patch which updates all of the
> tests.
> 
> > --- /dev/null
> > +++ b/t/t5100/quoted-pair.in
> > @@ -0,0 +1,9 @@
> > +From 1234567890123456789012345678901234567890 Mon Sep 17 00:00:00 2001
> > +From: "Author \"The Author\" Name" <somebody@example.com>
> > +Date: Sun, 25 May 2008 00:38:18 -0700
> > +Subject: [PATCH] testing quoted-pair
> 
> I do not care that much about the "()" comment behavior myself, but if
> we are going to implement it, it probably makes sense to protect it from
> regression with a test.

Yeah, good idea.
> 
> -Peff

^ permalink raw reply

* Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]
From: Andrew Donnellan @ 2016-09-19  9:17 UTC (permalink / raw)
  To: Josh Triplett, git; +Cc: Jeff King
In-Reply-To: <b5bf39015fdd20dd0aa4f38eb365bbbd0d07a4ca.1474096535.git-series.josh@joshtriplett.org>

On 17/09/16 17:21, Josh Triplett wrote:
> This provides a shorter and more convenient alias for
> --subject-prefix='RFC PATCH'.
>
> Includes documentation in the format-patch manpage, and a new test
> covering --rfc.
>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>

Sounds good to me. Agreed that "RFC" is essentially the only prefix 
other than "PATCH" that I see, at least in the kernel.

I don't have anything to say about the code, though I did note that 
there's a error message stating that "--subject-prefix and -k are 
mutually exclusive." - I haven't tested the patch, but I imagine this 
message will trigger with --rfc as well and could be slightly confusing.

> ---
> v2:
> - Add documentation to the format-patch manpage
> - Call subject_prefix_callback rather than reimplementing it
> - Update test to move expectations inside
>
>  Documentation/git-format-patch.txt |  8 +++++++-
>  builtin/log.c                      |  8 ++++++++
>  t/t4014-format-patch.sh            |  9 +++++++++
>  3 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 9624c84..b9590a5 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -19,7 +19,8 @@ SYNOPSIS
>  		   [--start-number <n>] [--numbered-files]
>  		   [--in-reply-to=Message-Id] [--suffix=.<sfx>]
>  		   [--ignore-if-in-upstream]
> -		   [--subject-prefix=Subject-Prefix] [(--reroll-count|-v) <n>]
> +		   [--rfc] [--subject-prefix=Subject-Prefix]
> +		   [(--reroll-count|-v) <n>]
>  		   [--to=<email>] [--cc=<email>]
>  		   [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
>  		   [<common diff options>]
> @@ -172,6 +173,11 @@ will want to ensure that threading is disabled for `git send-email`.
>  	allows for useful naming of a patch series, and can be
>  	combined with the `--numbered` option.
>
> +--rfc::
> +	Alias for `--subject-prefix="RFC PATCH"`. Use this when
> +	sending an experimental patch for discussion rather than
> +	application.

Perhaps mention the phrase "Request For Comment" for the benefit of 
those who aren't familiar (which admittedly, among users of 
git-format-patch, are probably rather few, but still).

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited


^ permalink raw reply

* Re: [PATCH v3 0/8] Better heuristics make prettier diffs
From: Michael Haggerty @ 2016-09-19  6:35 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Ramsay Jones, René Scharfe, Stefan Beller, Jeff King,
	Jakub Narębski, Jacob Keller
In-Reply-To: <xmqqr38vjns0.fsf@gitster.mtv.corp.google.com>

On 09/08/2016 01:25 AM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>>   * Add test t4059 as part of this commit, not as part of its
>>     successor.
> 
> Which needs to be moved to somewhere else, as another topics that
> has already been in 'next' uses t4059.
> 
> I'd move it temporarily to t4061 with a separate SQUASH??? at the
> tip for now, as I am running out of time today.

I didn't realize you were waiting for an ACK. Yes, it's totally OK to
rename the test.

Michael


^ permalink raw reply

* Re: Finding a commit based on the diff index line?
From: Philip Oakley @ 2016-09-18 19:37 UTC (permalink / raw)
  To: Philip Oakley, Junio C Hamano; +Cc: Git List
In-Reply-To: <FD07B781038A4C878F6D0152A4ECE580@PhilipOakley>

From: "Philip Oakley" <philipoakley@iee.org>
>> From: Junio C Hamano
>> Sent: Saturday, September 17, 2016 5:30 PM
>>
>> Does it count as simple to use pager's search feature in
>> "log --raw" output piped to it?
>>
>> Pardon typos & html; typed on a phone
>>
>
> The `log --raw` looks useful, though I think (IIUC) I also have a problem 
> that (obviously?) these commits are not linked behind any existing ref 
> (except maybe the reflogs which I'm not up to speed on).
>
For others that may be reading... The command I eventually ended up using to 
find old series was:

git reflog HEAD --grep-reflog='rebase -i (finish)' | cut -c1-7 | xargs -n1 
git quote

where the alias.quote = log -1 --pretty='tformat:%h (%s, %ad)' --date=short

I can then compare the dates of my corrective rebases to the patch 
submission dates to find the (local sha1 of) missing series.

> I had somehow hoped for some neat magic command or rev specifier that took 
> the revs as alternates to the `-- <paths>` in rev-parse or some such....

Though locating a commit based on a diff's index line does still feel to be 
something that should be possible.
>
> It's tricky curating old mistakes ;-)
>
> Thanks for the pointer.
>
>> On Sep 17, 2016 07:33, "Philip Oakley" <philipoakley@iee.org> wrote:
>>
>> > Hi,
>> >
>> > I'm curating some of my old patch series (i.e. doing some tidying up) 
>> > and
>> > I'm trying to determine the commits that generated some of my patches 
>> > so
>> > that I can see if I still have them after they were rebased (a 'name 
>> > that
>> > dangling branch' problem).
>> >
>> > Is there an easy way of finding the commit sha1 that contains the given 
>> > diff
>> > index line.
>> >
>> > For example.
>> > index fa05269..57033dd 100755
>> > or
>> > index 8ebcded..d9ab360 100644
>> >
>> > which both should get back to Jeff King's 36d6792 (t0006: test various 
>> > date
>> > formats, 2016-06-20).
>> >
>> > It feels like it is something that should already possible without a
>> > mini-script. We have the rev range which should limit the range to a 
>> > single
>> > commit, though if random blob revs were given the commit range would be
>> > 'scattered'.
>> >
>> >
>> > Is there a simple quick way of achieving this?
>> >
>> > --
>> > Philip
>
> 


^ permalink raw reply

* Re: Gitattributes file is not respected when switching between branches
From: Torsten Bögershausen @ 2016-09-18 19:31 UTC (permalink / raw)
  To: Виталий Ищенко,
	Torsten Bögershausen
  Cc: git
In-Reply-To: <CANYoZJnB81rEGNAjGj6jOscmdSW_niSy6jRP6acw2xB4ssX8xA@mail.gmail.com>

On 16.09.16 08:51, Виталий Ищенко wrote:
> Sorry for delay.
> 
No problem about the delay.

(And please no top-posting)


If you say
> ".gitattributes" indeed is not present in "master", but this is intentionally
then nobody has (to my knowledge) thought about this situation/workflow yet.


The short version:
Git is designed to have the same .gitattributes in different branches.
At least not in the long run.

A typical use case is to create a repo, adjust the
.gitattributes and keep this in all branches.



 




^ permalink raw reply

* Re: [PATCH 10/11] run command: add RUN_COMMAND_NO_STDOUT
From: Johannes Schindelin @ 2016-09-18 19:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kevin Wern, git
In-Reply-To: <xmqq37kzigvf.fsf@gitster.mtv.corp.google.com>

Hi,

On Fri, 16 Sep 2016, Junio C Hamano wrote:

> Kevin Wern <kevin.m.wern@gmail.com> writes:
> 
> > Add option RUN_COMMAND_NO_STDOUT, which sets no_stdout on a child
> > process.
> >
> > This will be used by git clone when calling index-pack on a downloaded
> > packfile.
> 
> If it is just one caller, would't it make more sense for that caller
> set no_stdout explicitly itself?

Taking a step back, maybe it is not such a good idea to swallow the output
in all cases, including the error cases?

Maybe the best course of action is to hide stdout/stderr by default but
show it in case of a non-zero exit code, i.e. using
https://public-inbox.org/git/6383b7afcdeb6c999862aa32ba437997f2dd3d4e.1472633606.git.johannes.schindelin@gmx.de/ ?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] ls-files: add pathspec matching for submodules
From: Brandon Williams @ 2016-09-18 18:40 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Heiko Voigt, Nguyễn Thái Ngọc Duy,
	Stefan Beller
In-Reply-To: <xmqqtwdfgpd0.fsf@gitster.mtv.corp.google.com>

On Fri, Sep 16, 2016 at 8:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> As the previous one that used a wrong (sorry) argument is not even
> in 'next' yet, let's pretend that it never happened.  It is OK to
> still keep it and this patch as two separate steps, i.e. a topic
> with two patches in it.

Yeah since I've been using what I built in the last patch to test the pathspecs
I based this patch off of what you have in bw/ls-files... branch.

>
>> +     /* Add pathspec args */
>> +     argv_array_push(&cp.args, "--");
>> +     for (i = 0; i < pathspec.nr; ++i)
>> +             argv_array_push(&cp.args, pathspec.items[i].original);
>
> OK, so as discussed previously with Heiko and Stefan, the idea is to
>
>  - pass the original pathspec as-is,
>
>  - when --submodule-prefix is given, a path discovered in a
>    submodule repository is first prefixed with that string before
>    getting checked to see if it matches the original pathspec.
>
> And this loop is about relaying the original pathspec.

Exactly.  Perhaps I should have made this more clear either with a
detailed comment or
more information in the commit msg.

>> @@ -192,57 +210,63 @@ static void show_gitlink(const struct cache_entry *ce)
>>
>>  static void show_ce_entry(const char *tag, const struct cache_entry *ce)
>>  {
>> +     struct strbuf name = STRBUF_INIT;
>>       int len = max_prefix_len;
>> +     if (submodule_prefix)
>> +             strbuf_addstr(&name, submodule_prefix);
>> +     strbuf_addstr(&name, ce->name);
>>
>>       if (len >= ce_namelen(ce))
>> -             die("git ls-files: internal error - cache entry not superset of prefix");
>> +             die("git ls-files: internal error - cache entry not "
>> +                 "superset of prefix");
>
> This is not such a great thing to do.  Upon a bug report, we can no
> longer do
>
>         git grep 'cache entry not superset'
>
> to see where the error message is coming from.

Oh i wasn't really thinking about that.  I simply noticed that the
line was longer than
80 characters and figured I should break it to adhere to style guidelines.

>
>> -     if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
>> -                         len, ps_matched,
>> -                         S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
>> -             return;
>> -     if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
>> +     if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
>> +         submodule_path_match(&pathspec, name.buf, ps_matched)) {
>>               show_gitlink(ce);
>> -             return;
>> -     }
>> +     } else if (match_pathspec(&pathspec, name.buf, name.len,
>> +                               len, ps_matched,
>> +                               S_ISDIR(ce->ce_mode) ||
>> +                               S_ISGITLINK(ce->ce_mode))) {
>> +             if (tag && *tag && show_valid_bit &&
>> + ...
>
> Argh.  If we had a preparatory clean-up step, would it have helped
> to avoid this big re-indentation that makes the patch harder to read
> than necessary, I wonder?
>
> Another way would have been to "goto" from the end of this block
>
>> +     if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
>> +         submodule_path_match(&pathspec, name.buf, ps_matched)) {
>
> where we used to "return" out to the central clean-up location, i.e.
> here.
>
>> +     strbuf_release(&name);
>>  }

Yeah, the lack of destructors in C makes this a bit of a challenge when we need
to free up memory.  I've also always been taught to avoid using goto's
as they can
be error prone and lead to making the code more difficult to read.
Hence why I did
 some adjustments to make it so the function had a single exit point
to make it easy
 for cleanup.


>
>
>>       parse_pathspec(&pathspec, 0,
>>                      PATHSPEC_PREFER_CWD |
>>                      PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
>>                      prefix, argv);
>>
>> -     /* Find common prefix for all pathspec's */
>> -     max_prefix = common_prefix(&pathspec);
>> +     /*
>> +      * Find common prefix for all pathspec's
>> +      * This is used as a performance optimization which violates correctness
>> +      * in the recurse_submodules mode
>> +      */
>
> The two new lines phrase it overly negatively and also misleading.
> I thought you were saying "We do this as optimization anyway; damn
> the correctness in the submodule case!" in my first reading before
> reading the statements the comment talks about.  "This optimization
> unfortunately cannot be done when recursing into submodules" would
> have been better.

haha yeah, I wasn't trying to be overly negative!  I agree that your
wording makes more sense.

>
>> +     if (recurse_submodules)
>> +             max_prefix = NULL;
>> +     else
>> +             max_prefix = common_prefix(&pathspec);
>>       max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
>
>> diff --git a/dir.c b/dir.c
>> index 0ea235f..630dc7a 100644
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -63,6 +63,30 @@ int fspathncmp(const char *a, const char *b, size_t count)
>>       return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
>>  }
>>
>> +static int prefix_fnmatch(const struct pathspec_item *item,
>> +                const char *pattern, const char *string,
>> +                int prefix)
>> +{
>
> Is this meant to be free of false positives, free of false
> negatives, or exact?  I think you use it to decide, without knowing
> what kind of paths the submodule contains, if it is worth descending
> into it, so as long as you definitively say "The pathspec can never
> match anything in the submodule" with WM_NOMATCH, it is OK if you
> returned WM_MATCH when it actually couldn't match anything.  I.e. it
> is OK to give false positive but it is a bug to give false negative.
>
> The answer to the above question should be a good explanation to
> prepend as /* comment */ before the function.

Yeah the idea is the parent project has no knowledge of the file
structure of the
child so if the submodule name matches a prefix of the pathspec then it could
potentially apply to a file inside the submodule.  So it can
definitely give false
positives since we could descend into the submodule and discover that it doesn't
match anything there.  I can add a comment to that effect.

>
>> +     if (prefix > 0) {
>> +             if (ps_strncmp(item, pattern, string, prefix))
>> +                     return WM_NOMATCH;
>
> This says: when we have a set prefix that must literally match, and
> that part does not match what we have, it cannot possibly match.
>
> Is that correct?  What do we have in "name" and "item" at this
> point?  We disable the common-prefix optimization, so we do not have
> to worry about a pathspec with two elements "sub/dir1/*" and "sub/dir2/*"
> giving you "sub/dir" as the common prefix, when you are wondering if
> it is worth descending into "sub/" without knowing what it contains.
> Is that what guarantees why this part is correct?

I adopted this structure from another part of the code.  The caller
uses a field in
the pathspec item which indicates the location of the first wildcard character.
So the prefix (everything prior to the wildcard char) must match
literally before
we drop into a more expensive wildmatch function.


>
>> +             pattern += prefix;
>> +             string += prefix;
>> +     }
>> +
>> +     if (item->flags & PATHSPEC_ONESTAR) {
>> +             return WM_MATCH;
>
> We have a pathspec that has a segment without wildcard letters,
> followed by a '*', and there is no wildcard letters after that
> asterisk.  We punt and assume it might match, which is OK for the
> purpose of not giving a false negative.

Exactly.  Currently if you give *.txt as a pathspec it can match every
txt file no
matter which directory its in.  Because of this if we hit a * we can
punt and hand
it off to the submodule to perform more accurate matching.

>
>> +     } else if (item->magic & PATHSPEC_GLOB) {
>> +             return wildmatch(pattern, string,
>> +                              WM_PATHNAME |
>> +                              (item->magic & PATHSPEC_ICASE ?
>> +                               WM_CASEFOLD : 0),
>> +                              NULL);
>
> What does this say?  If we are using the :(glob) semantics, which is
> the default, we'll ask wildmatch() to see the remainder of the
> pattern (after stripping the fixed prefix part if necessary) matches
> the string (which also may have lost the prefix that we already know
> matches).
>
> Is that correct?  I think it depends on what "string" is being fed,
> but I am assuing that you are working in the top-level project here
> to decide if it is worth descending into a submodule.  If the item
> is sub/dir?/*.c and we are considering "sub/" submodule, wildmatch
> would not say "It could match" if "string" is "sub/".  Perhaps I am
> reading the patch incorrectly.  Let me read on to see what the caller
> does later.

Actually thinking on this we may need to add in a special case to the
wildmatch function.
Since right now I believe that a string must completely match against
the pattern passed
in to wildmatch as apposed to wanting the string to match a prefix of
the pattern.

-Brandon

^ permalink raw reply

* Re: Bug: pager.<cmd> doesn't work well with editors
From: Anatoly Borodin @ 2016-09-18 15:51 UTC (permalink / raw)
  To: git
In-Reply-To: <nrmbrl$hsk$1@blaine.gmane.org>

> I think, the pagination should be turned off when the editor is being
> called.

... even if the `[-p|--paginate]` option is used explicitly. Is there a
case when pagination shouldn't be ignored with an editor?

`git -p -c core.editor=gvim config -e` works, but the pagination is not
effective.

`git -p -c core.editor=vim config -e` doesn't work and wants to be killed.

I've tested it on FreeBSD and Linux, have no idea, how it works on Mac or
Windows.

-- 
Mit freundlichen Grüßen,
Anatoly Borodin


^ permalink raw reply

* Homebrew and Git
From: Jonas Thiel @ 2016-09-18 15:50 UTC (permalink / raw)
  To: git

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

Dear Sir or Madame,
 
A while ago I have described my problem with Homebrew at the following GitHub channel (https://github.com/Homebrew/homebrew-core/issues/2970). In the meanwhile, I believe that I my problem with Homebrew is based on an issues with my Git. I have found the attached Git Crash reports on my Mac and because I am not familiar with reading/analysing Crash Reports, it would be great if someone could give me some feedback on it.
 
If you have any question, please do not hesitate to contact me.
 
Thanks a lot in advance and best regards,
Jonas

[-- Attachment #2: Git.Crash.report.zip --]
[-- Type: application/zip, Size: 13686 bytes --]

^ permalink raw reply

* Bug: pager.<cmd> doesn't work well with editors
From: Anatoly Borodin @ 2016-09-18 15:28 UTC (permalink / raw)
  To: git

Hi All!

It can be useful to enable `pager.branch`, `pager.tag`, `pager.config`, etc
for some projects (`git` itself can be a good example, with all its feature
branches and tags).

But it makes commands like `git branch --edit-description`, `git tag -a`,
`git config -e` extremely unhappy. For example, `vim` says

	Vim: Warning: Output is not to a terminal

and then becomes unusable (just as `vim | less` would be).

I think, the pagination should be turned off when the editor is being
called.

-- 
Mit freundlichen Grüßen,
Anatoly Borodin


^ permalink raw reply

* Re: Two bugs in --pretty with %C(auto)
From: René Scharfe @ 2016-09-18 13:21 UTC (permalink / raw)
  To: Anatoly Borodin; +Cc: Duy Nguyen, git, Junio C Hamano
In-Reply-To: <CACNzp2mkD7Se9R=SVwXhyRstkGAs1N9YbNgaZRQYKkpPM1kj8A@mail.gmail.com>

Am 18.09.2016 um 14:30 schrieb Anatoly Borodin:
> On Sat, Sep 17, 2016 at 8:25 PM, René Scharfe <l.s.r@web.de> wrote:
>> I'm not sure how just how automatic %C(auto) is supposed to be, but you
>> expected it do emit the reset for you, right?  Sounds reasonable to me.
>
> I don't see a good reason not to do so. Spare some bytes?..

The states for colors and attributes are separate; that's how the bold 
attribute bled into your the auto-colored parts of your output.  You 
could use that property to specify e.g. "give me automatic coloring, but 
reverse it".

This only works by accident now, I think.  Full resets are emitted after 
many placeholders, so attributes don't reach very far in practice.  We'd 
have to be more careful with these full resets if we'd want attributes 
to cover multiple placeholders.  An automatic reset at the start of 
%C(auto) would go into the opposite direction.

René

^ permalink raw reply

* Re: Two bugs in --pretty with %C(auto)
From: Anatoly Borodin @ 2016-09-18 12:30 UTC (permalink / raw)
  To: René Scharfe; +Cc: Duy Nguyen, git, Junio C Hamano
In-Reply-To: <db20ae0c-9c33-1e65-b201-1b6a9ed11340@web.de>

Hi René!


On Sat, Sep 17, 2016 at 8:25 PM, René Scharfe <l.s.r@web.de> wrote:
> I'm not sure how just how automatic %C(auto) is supposed to be, but you
> expected it do emit the reset for you, right?  Sounds reasonable to me.

I don't see a good reason not to do so. Spare some bytes?..

> The following patch implements that behavior.

Thanks, the patch works great!


-- 
Mit freundlichen Grüßen,
Anatoly Borodin

^ permalink raw reply

* Re: Why are there multiple ways to get the manual in Git?
From: Philip Oakley @ 2016-09-18 10:51 UTC (permalink / raw)
  To: Fredrik Gustafsson, Andrew Johnson, Jakub Narębski
  Cc: git, Christian Couder
In-Reply-To: <223e818d-6d10-2679-c2d0-150011e74a59@gmail.com>

From: "Jakub Narębski" <jnareb@gmail.com>
>W dniu 17.09.2016 o 20:39, Fredrik Gustafsson pisze:
>> On Sat, Sep 17, 2016 at 01:47:52PM -0400, Andrew Johnson wrote:
>
>>> $ git help <verb>
>>> $ git <verb> --help
>>> $ man git-<verb>
>>>
>>> I tested all three to confirm they were equivalent.
>>
>> While I'm not able to answer your question, I can shred a little light
>> about them not being equal. For example using a windows machine
>>
>> $ man git-<verb>
>>
>> does not work and
>>
>> $ git help <verb>
>>
>> opens a webbrowser instead of a man page. Using a unix system I would
>> however assume that
>>
>> $ man git-<verb>
>>
>> would work since it's the standard way of getting help on those systems.
>
> There is also additional difference.  There are help pages which are
> not about specific Git command, but about concepts (gitcli, gitrevisions,
> githooks, gitrepository-layout, gitglossary), or about files (gitignore,
> gitattributes, to some extent githooks).
>
> Those are only accessible with `git help <concept>` or, on OS with
> installed 'man', also `man <gitconcept>`.

The `git revisions --help` does work ;-) But like you say, its apparent 
"command" name is 'gitrevisions'.

Thus real commands gave a git-command name, while concepts have a gitconcept 
name which can then be found via the man command.

--
Philip 


^ permalink raw reply

* Re: Why are there multiple ways to get the manual in Git?
From: Jakub Narębski @ 2016-09-18 10:21 UTC (permalink / raw)
  To: Fredrik Gustafsson, Andrew Johnson; +Cc: git, Christian Couder, Philip Oakley
In-Reply-To: <20160917183919.GJ20666@paksenarrion.iveqy.com>

W dniu 17.09.2016 o 20:39, Fredrik Gustafsson pisze:
> On Sat, Sep 17, 2016 at 01:47:52PM -0400, Andrew Johnson wrote:

>> $ git help <verb>
>> $ git <verb> --help
>> $ man git-<verb>
>>
>> I tested all three to confirm they were equivalent.
> 
> While I'm not able to answer your question, I can shred a little light
> about them not being equal. For example using a windows machine
> 
> $ man git-<verb>
> 
> does not work and
> 
> $ git help <verb>
> 
> opens a webbrowser instead of a man page. Using a unix system I would
> however assume that
> 
> $ man git-<verb>
> 
> would work since it's the standard way of getting help on those systems.

There is also additional difference.  There are help pages which are
not about specific Git command, but about concepts (gitcli, gitrevisions,
githooks, gitrepository-layout, gitglossary), or about files (gitignore,
gitattributes, to some extent githooks).

Those are only accessible with `git help <concept>` or, on OS with
installed 'man', also `man <gitconcept>`.

Just FYI
-- 
Jakub Narębski

^ permalink raw reply

* Re: Finding a commit based on the diff index line?
From: Philip Oakley @ 2016-09-17 19:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List
In-Reply-To: <CAPc5daUdM4wwY7C_YKx0DkW3kty+VewksMMXG39bBr1K-JxBag@mail.gmail.com>

> From: Junio C Hamano
> Sent: Saturday, September 17, 2016 5:30 PM
>
> Does it count as simple to use pager's search feature in
> "log --raw" output piped to it?
>
> Pardon typos & html; typed on a phone
>

The `log --raw` looks useful, though I think (IIUC) I also have a problem 
that (obviously?) these commits are not linked behind any existing ref 
(except maybe the reflogs which I'm not up to speed on).

I had somehow hoped for some neat magic command or rev specifier that took 
the revs as alternates to the `-- <paths>` in rev-parse or some such....

It's tricky curating old mistakes ;-)

Thanks for the pointer.

> On Sep 17, 2016 07:33, "Philip Oakley" <philipoakley@iee.org> wrote:
>
> > Hi,
> >
> > I'm curating some of my old patch series (i.e. doing some tidying up) 
> > and
> > I'm trying to determine the commits that generated some of my patches so
> > that I can see if I still have them after they were rebased (a 'name 
> > that
> > dangling branch' problem).
> >
> > Is there an easy way of finding the commit sha1 that contains the given 
> > diff
> > index line.
> >
> > For example.
> > index fa05269..57033dd 100755
> > or
> > index 8ebcded..d9ab360 100644
> >
> > which both should get back to Jeff King's 36d6792 (t0006: test various 
> > date
> > formats, 2016-06-20).
> >
> > It feels like it is something that should already possible without a
> > mini-script. We have the rev range which should limit the range to a 
> > single
> > commit, though if random blob revs were given the commit range would be
> > 'scattered'.
> >
> >
> > Is there a simple quick way of achieving this?
> >
> > --
> > Philip


^ permalink raw reply

* Re: Why are there multiple ways to get the manual in Git?
From: Christian Couder @ 2016-09-17 19:21 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: Andrew Johnson, git
In-Reply-To: <20160917183919.GJ20666@paksenarrion.iveqy.com>

On Sat, Sep 17, 2016 at 8:39 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> On Sat, Sep 17, 2016 at 01:47:52PM -0400, Andrew Johnson wrote:
>> $ git help <verb>
>> $ git <verb> --help
>> $ man git-<verb>
>>
>> I tested all three to confirm they were equivalent.

Maybe it will be easier to understand if you try:

$ git help help

or

$ git help --help

or

$ man git-help

:-)

> While I'm not able to answer your question, I can shred a little light
> about them not being equal. For example using a windows machine
>
> $ man git <verb>

(It should be "git-<verb>" above.)

> does not work and
>
> $ git help <verb>
>
> opens a webbrowser instead of a man page.

Yeah, it is one of the reasons.
There are systems where "man" is not the standard ways to get help,
and text on the command line is not the prefered format for help
content.
So the "git help" command has different default depending on the OS to
better suit user expectations on each OS.
This way people can just be teached to use "git help" and that will do
something sensible everywhere.

> Using a unix system I would
> however assume that
>
> $ man git <verb>

(Again it should be "git-<verb>" above.)

> would work since it's the standard way of getting help on those systems.

Yeah, so we need that to work to make people happy on unix systems.

Another reason is that "git help" provides more configurability and
more features like its -a and -g options.
It could provide even more in the future, like options to search in
the documentation.

^ permalink raw reply

* Re: Why are there multiple ways to get the manual in Git?
From: Philip Oakley @ 2016-09-17 19:12 UTC (permalink / raw)
  To: Fredrik Gustafsson, Andrew Johnson; +Cc: git
In-Reply-To: <20160917183919.GJ20666@paksenarrion.iveqy.com>

From: "Fredrik Gustafsson" <iveqy@iveqy.com>
> On Sat, Sep 17, 2016 at 01:47:52PM -0400, Andrew Johnson wrote:
>> $ git help <verb>
>> $ git <verb> --help
>> $ man git-<verb>
>>
>> I tested all three to confirm they were equivalent.

It is (IIUC) in a general sort of way "by design", and a little bit of 
accident.

>
> While I'm not able to answer your question, I can shred a little light
> about them not being equal. For example using a windows machine
>
> $ man git <verb>
>
> does not work and
>
> $ git help <verb>
>
> opens a webbrowser instead of a man page. Using a unix system I would
> however assume that
>
> $ man git <verb>
>
> would work since it's the standard way of getting help on those systems.
>
> -- 

Historically git was a set of shell scripts named git-*, so each stood 
alone.

Then there was the great consolidation (around V1.6?) which created the 
modern `git <cmd>' approach, with every command normally having  -h 
and --help options for short form usage and long form man pages.

The option capability became standardised. Also a `git help <cmd>` command 
was created. Underneath there are still the (backward compatible) git-* 
forms. The help command allowed selection of display type, so that on 
Unix/Linux man was the norm, while an --html (or --web) option is available 
for those who like the pretty browser view

The help commnad just converts the parameters to achieve the expected 
display (with various fallbacks if the command or guide is missing, etc)

Meanwhile on Windows, the man facility was not ported as part of git, so it 
defaults to the --web version. If you are on Windows, and download the SDK 
as well you can install the man viewer and other goodies

--
Philip


^ permalink raw reply


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