* Re: [Q] would it be bad to make /etc/gitconfig runtime configurable?
From: Junio C Hamano @ 2016-10-03 16:24 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20161003112654.3vca4zmctslcudfz@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I admit both of those are uses for git _developers_, though, not git
> _users_.
Yes, this is meant for developers and not users.
The initial question probably should have stated more explicitly,
e.g. "I am wondering if it would be helpful to developers if we add
this thing; does anybody think of a reason why exposing it to end
users is a bad idea?"
^ permalink raw reply
* Re: [PATCH v2 6/6] git-gui: Update Japanese information
From: Junio C Hamano @ 2016-10-03 16:18 UTC (permalink / raw)
To: Pat Thoyts; +Cc: Satoshi Yasushima, git, Jakub Narębski
In-Reply-To: <87int9x1lp.fsf@red.patthoyts.tk>
Pat Thoyts <patthoyts@users.sourceforge.net> writes:
> I'm just starting to catch up once again. hopefully I can be
> a bit more reactive than recently. Merging 52285c83 looks fine. I'll
> stick that onto the 0.20.0 head and see what else I can pick up on top.
> There are a few from the git for windows set among others.
Nice to hear from you again. I think I have a few topics I merged
to my tree bypassing you in the meantime. Let me get back to you
with a list of topic tips to bring your tree in sync with what I
have later.
^ permalink raw reply
* Re: [RFC PATCH] clone: add clone.recursesubmodules config option
From: Jeremy Morton @ 2016-10-03 15:36 UTC (permalink / raw)
To: Chris Packham; +Cc: git, mara.kim, gitster
In-Reply-To: <1401874256-13332-1-git-send-email-judge.packham@gmail.com>
Did this ever get anywhere? Can we recursively update submodules with
"git pull" in the supermodule now?
--
Best regards,
Jeremy Morton (Jez)
On 04/06/2014 10:30, Chris Packham wrote:
> Add a config option that will cause clone to recurse into submodules as
> if the --recurse-submodules option had been specified on the command
> line. This can be overridden with the --no-recurse-submodules option.
>
> Signed-off-by: Chris Packham<judge.packham@gmail.com>
> ---
> On 04/06/14 09:05, Junio C Hamano wrote:
>>> Mara Kim<mara.kim@vanderbilt.edu> writes:
>>>
>>>> Apologies if this question has been asked already, but what is the
>>>> reasoning behind making git clone not recursive (--recursive) by
>>>> default?
>>>
>>> The primary reason why submodules are separate repositories is not
>>> to require people to have everything. Some people want recursive,
>>> some others don't, and the world is not always "majority wins" (not
>>> that I am saying that majority will want recursive).
>>>
>>> Inertia, aka backward compatibility and not surprising existing
>>> users, plays some role when deciding the default.
>>>
>>> Also, going --recursive when the user did not want is a lot more
>>> expensive mistake to fix than not being --recursive when the user
>>> wanted to.
>>
>> Having said all that, I do not mean to say that I am opposed to
>> introduce some mechanism to let the users express their preference
>> between recursive and non-recursive better, so that "git clone"
>> without an explicit --recursive (or --no-recursive) can work to
>> their taste. A configuration in $HOME/.gitconfig might be a place
>> to start, even though that has the downside of assuming that the
>> given user would want to use the same settings for all his projects,
>> which may not be the case in practice.
>
> And here's a quick proof of concept. Not sure about the config variable name
> and it could probably do with a negative test as well.
>
> builtin/clone.c | 9 +++++++++
> t/t7407-submodule-foreach.sh | 17 +++++++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index b12989d..92aea81 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -734,6 +734,14 @@ static void write_refspec_config(const char* src_ref_prefix,
> strbuf_release(&value);
> }
>
> +static int git_clone_config(const char *key, const char *value, void *data)
> +{
> + if (!strcmp(key, "clone.recursesubmodules"))
> + option_recursive = git_config_bool(key, value);
> +
> + return 0;
> +}
> +
> int cmd_clone(int argc, const char **argv, const char *prefix)
> {
> int is_bundle = 0, is_local;
> @@ -759,6 +767,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> junk_pid = getpid();
>
> packet_trace_identity("clone");
> + git_config(git_clone_config, NULL);
> argc = parse_options(argc, argv, prefix, builtin_clone_options,
> builtin_clone_usage, 0);
>
> diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
> index 7ca10b8..fc2c189 100755
> --- a/t/t7407-submodule-foreach.sh
> +++ b/t/t7407-submodule-foreach.sh
> @@ -307,6 +307,23 @@ test_expect_success 'use "update --recursive nested1" to checkout all submodules
> )
> '
>
> +test_expect_success 'use "git clone" with clone.recursesubmodules to checkout all submodules' '
> + git config --local clone.recursesubmodules true&&
> + git clone super clone7&&
> + (
> + cd clone7&&
> + git rev-parse --resolve-git-dir .git&&
> + git rev-parse --resolve-git-dir sub1/.git&&
> + git rev-parse --resolve-git-dir sub2/.git&&
> + git rev-parse --resolve-git-dir sub3/.git&&
> + git rev-parse --resolve-git-dir nested1/.git&&
> + git rev-parse --resolve-git-dir nested1/nested2/.git&&
> + git rev-parse --resolve-git-dir nested1/nested2/nested3/.git&&
> + git rev-parse --resolve-git-dir nested1/nested2/nested3/submodule/.git
> + )&&
> + git config --local --unset clone.recursesubmodules
> +'
> +
> test_expect_success 'command passed to foreach retains notion of stdin' '
> (
> cd super&&
^ permalink raw reply
* Re: [RFC/PATCH 0/2] place cherry pick line below commit title
From: Junio C Hamano @ 2016-10-03 15:23 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, Christian Couder
In-Reply-To: <42332581-e47a-0fc8-ed5c-44e7e1c19341@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> On 09/30/2016 12:34 PM, Junio C Hamano wrote:
>>> 2) The Linux kernel's repository has some "commit ... upstream." lines
>>> in this position (below the commit title) - for example, in commit
>>> dacc0987fd2e.
>>
>> "A group of people seem to prefer it there" does not lead to
>> "therefore let's move it there for everybody". It does open a
>> possibility that we may want to add a new option to put it there,
>> but does not justify changing what existing "-x" option does.
>
> To clarify, my patch adds the new option you described (to place it
> below the title instead of at the bottom of the commit message). The
> default is still the current behavior.
Ah, sorry, I missed that. No objection from me on this point then.
Thanks.
^ permalink raw reply
* Re: Feature Request: user defined suffix for temp files created by git-mergetool
From: Anatoly Borodin @ 2016-10-03 15:18 UTC (permalink / raw)
To: Josef Ridky; +Cc: git
In-Reply-To: <1329039097.128066.1475476591437.JavaMail.zimbra@redhat.com>
Hi Josef,
On Mon, Oct 3, 2016 at 8:36 AM, Josef Ridky <jridky@redhat.com> wrote:
> In several projects, we are using git mergetool for comparing files from different folders.
> Unfortunately, when we have opened three files for comparing using meld tool (e.q. Old_version -- Result -- New_version),
> we can see only name of temporary files created by mergetool in the labels (e.g. foo_REMOTE -- foo_BASE -- foo_LOCAL)
> and users (and sometime even we) are confused, which of the files should they edit and save.
`git mergetool` just creates temporary files (with some temporary
names) and calls `meld` (or `vimdiff`, etc) with the file names as
parameters. So why wouldn't you call `meld` with the file names you
want?
--
Mit freundlichen Grüßen,
Anatoly Borodin
^ permalink raw reply
* Re: [PATCH v3 0/5] Add --format to tag verification
From: Santiago Torres @ 2016-10-03 14:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, peff, sunshine, walters
In-Reply-To: <xmqqlgy97ys0.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 374 bytes --]
Hi, Junio.
> I however notice that there is no new tests to protect these two new
> features from future breakages. Perhaps you want to add some in
> [6/5]?
I'll be working on this. I spent some time looking around for example
tests for format. Are there any that I should pay special attention to?
(I'm looking at t7004 mostly right now).
Thanks!
-Santiago.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v2 02/11] i18n: add--interactive: mark simple here documents for translation
From: Vasco Almeida @ 2016-10-03 13:21 UTC (permalink / raw)
To: Jakub Narębski, Junio C Hamano, git
Cc: Jiang Xin, Ævar Arnfjörð Bjarmason, David Aguilar
In-Reply-To: <e71c5040-8fed-887b-bf35-af4ae28d469f@gmail.com>
A Sex, 30-09-2016 às 19:26 +0200, Jakub Narębski escreveu:
> W dniu 31.08.2016 o 14:31, Vasco Almeida pisze:
> > Mark messages in here document without interpolation for
> translation.
> >
> > Marking for translation by removing here documents this way, rather
> than
> > take advantage of "print __ << EOF" way, makes other instances of
> help
> > messages in clean.c match the first two in this file. Otherwise,
> > reusing here document would add a trailer newline to the message,
> making
> > them not match 100%, hence creating two entries in pot template for
> > translation rather than a single entry.
>
> This is good catch, but I think it goes backwards with the solution.
>
> If the text to be translated is multi-line, and it must end with
> newline,
> why is this final newline not included in the msgid? This would
> involve
> turning printf_ln into printf, and adding trailing newline in final
> entry for builtin/clean.c:295, etc. - I think it is better solution
> than
> uglyifing git-add--interactive.perl
>
> Though it is not much of uglifying thanks to Perl support for
> embedded
> newlines in double-quoted strings.
I will do this for the next re-roll. Thanks
^ permalink raw reply
* Re: [PATCH 1/5] git-gui i18n: mark strings for translation
From: Vasco Almeida @ 2016-10-03 12:57 UTC (permalink / raw)
To: patthoyts; +Cc: git, Shawn O . Pearce
In-Reply-To: <8760p9wwr3.fsf@red.patthoyts.tk>
A Seg, 03-10-2016 às 11:30 +0100, Pat Thoyts escreveu:
> Vasco Almeida <vascomalmeida@sapo.pt> writes:
>
> >
> > Mark strings for translation in lib/index.tcl that were seemingly
> > left behind by 700e560 ("git-gui: Mark forgotten strings for
> > translation.", 2008-09-04) which marks string in
> > do_revert_selection
> > procedure.
> > These strings are passed to unstage_help and add_helper procedures.
> >
> > Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
> > ---
> > lib/index.tcl | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/index.tcl b/lib/index.tcl
> > index 74a81a7..3a3e534 100644
> > --- a/lib/index.tcl
> > +++ b/lib/index.tcl
> > @@ -291,7 +291,7 @@ proc do_unstage_selection {} {
> >
> > if {[array size selected_paths] > 0} {
> > unstage_helper \
> > - {Unstaging selected files from commit} \
> > + [mc "Unstaging selected files from
> > commit"] \
> > [array names selected_paths]
> > } elseif {$current_diff_path ne {}} {
> > unstage_helper \
> > @@ -343,7 +343,7 @@ proc do_add_selection {} {
> >
> > if {[array size selected_paths] > 0} {
> > add_helper \
> > - {Adding selected files} \
> > + [mc "Adding selected files"] \
> > [array names selected_paths]
> > } elseif {$current_diff_path ne {}} {
> > add_helper \
> > @@ -385,7 +385,7 @@ proc do_add_all {} {
> > set paths [concat $paths $untracked_paths]
> > }
> > }
> > - add_helper {Adding all changed files} $paths
> > + add_helper [mc "Adding all changed files"] $paths
> > }
> >
> > proc revert_helper {txt paths} {
>
> This series looks good. Especially noticing the misuse of the append
> command. Applied to my pu for now.
> Thank you,
I think Junio Hamano added PATCH 1/5 of this series to Git 'next'
branch and added the Portuguese translation of git-gui too.
^ permalink raw reply
* Re: [PATCH v2 05/11] i18n: add--interactive: mark message for translation
From: Vasco Almeida @ 2016-10-03 12:49 UTC (permalink / raw)
To: Jakub Narębski, Junio C Hamano
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
David Aguilar
In-Reply-To: <32b8c9ed-82df-0d71-a610-064b6ebf7a4e@gmail.com>
A Sáb, 01-10-2016 às 19:09 +0200, Jakub Narębski escreveu:
> W dniu 26.09.2016 o 01:09, Junio C Hamano pisze:
> > Vasco Almeida <vascomalmeida@sapo.pt> writes:
> >
> >> - print colored $prompt_color,
> $patch_mode_flavour{VERB},
> >> - ($hunk[$ix]{TYPE} eq 'mode' ? ' mode change' :
> >> - $hunk[$ix]{TYPE} eq 'deletion' ? ' deletion' :
> >> - ' this hunk'),
> >> - $patch_mode_flavour{TARGET},
> >> - " [y,n,q,a,d,/$other,?]? ";
> >
> > I hate to say this but expanding this single-liner into if/elsif/
> > cascade of uncountable number of arms is simply a disaster.
>
> Even if we turn this "single"-liner composition of sentence into
> interpolation (allowing for reordering of parts in translation),
> like
>
> print colored $prompt_color, __x("{verb} {noun}{maybe_target}
> [y,n,q,a,d,/{other},?]? ",
> verb => $patch_mode_flavour{VERB}, noun =>
> $patch_mode_noun{$hunk[$ix]{TYPE}},
> maybe_target => $patch_mode_flavour{TARGET} || "", other =>
> $other);
>
> This would of course require N__() on values of hash, somewhere.
>
> the problem is that the ordering may need to change depending on
> verb: "Stage", "Stash", "Unstage", "Apply", "Discard", and/or noun:
> "mode change", "deletion", "this hunk", and/or presence and value
> of maybe_target: " to index", " from worktree", " from index and
> worktree",
> " to index and worktree".
So it does not work, unfortunately. The plus side is it would be very
concise compared to laying every combination as entire sentences.
However, if it worked, I think it would be a bit difficult to translate
and translators would be prone to commit some mistake somewhere. It may
be harder to translate a sentence by its bits than translate it as a
whole.
^ permalink raw reply
* Re: [PATCH v2 04/11] i18n: add--interactive: mark plural strings
From: Vasco Almeida @ 2016-10-03 12:46 UTC (permalink / raw)
To: Jakub Narębski, Junio C Hamano, git
Cc: Jiang Xin, Ævar Arnfjörð Bjarmason, David Aguilar
In-Reply-To: <a46dc5cb-85d9-88f2-865c-734204832101@gmail.com>
A Sáb, 01-10-2016 às 18:49 +0200, Jakub Narębski escreveu:
> W dniu 26.09.2016 o 20:15, Vasco Almeida pisze:
> >
> > A Qua, 31-08-2016 às 12:31 +0000, Vasco Almeida escreveu:
> > >
> > >
> > > Mark plural strings for translation. Unfold each action case in
> > > one
> > > entire sentence.
> > >
> > > Pass new keyword for xgettext to extract.
> > >
> > > Update test to include new subrotine Q__() for plural strings
> > > handling.
>
> Why use Q__() as the name of the subroutine? [looks further]. Oh, I
> see
> that you are following the example of C shortcut functions (_, Q_ and
> N_).
>
> But this is Perl, not C. The standard shortcut functions are those
> defined in Locale::TextDomain, even if we can't use this module
> directly.
> Those that deal with plural strings handling are __n and __nx / __xn.
>
> The Perl equivalent of Q_ shorthand function in C, C++, etc. is __n.
> There is also a function __nx for combining handling plural strings
> together with variable interpolation.
I was trying to follow the same style of C. I will change the name Q__
to __x for the sake of conformance.
> > > diff --git a/git-add--interactive.perl b/git-add
> > > --interactive.perl
> > > index 4e1e857..08badfa 100755
> > > --- a/git-add--interactive.perl
> > > +++ b/git-add--interactive.perl
> > > @@ -666,12 +666,18 @@ sub status_cmd {
> > > sub say_n_paths {
> > > my $did = shift @_;
> > > my $cnt = scalar @_;
> > > - print "$did ";
> > > - if (1 < $cnt) {
> > > - print "$cnt paths\n";
> > > - }
> > > - else {
> > > - print "one path\n";
> > > + if ($did eq 'added') {
> > > + printf(Q__("added one path\n", "added %d
> > > paths\n",
> > > + $cnt), $cnt);
> > > + } elsif ($did eq 'updated') {
> > > + printf(Q__("updated one path\n", "updated %d
> > > paths\n",
> > > + $cnt), $cnt);
> > > + } elsif ($did eq 'reverted') {
> > > + printf(Q__("reverted one path\n", "reverted %d
> > > paths\n",
> > > + $cnt), $cnt);
> > > + } else {
> > > + printf(Q__("touched one path\n", "touched %d
> > > paths\n",
> > > + $cnt), $cnt);
> > > }
> > > }
>
> One one hand side, it is recommended to avoid lego-like construction
> of sentences.
>
> Translatable strings should be entire sentences. It is often not
> possible to translate single verbs or adjectives in a substitutable
> way.
>
> I think however that the action part ($did in original non-i18n code)
> is whole part in any language, so something like the following would
> be enough:
>
> # this hash is as much for validation, as for translation
> my %actions = map { $_ => 1 } (N__"added", N__"updated",
> N__"reverted");
> if (exists $actions{$did}) {
> print __nx("{did} one path\n", "{did} {count}
> paths\n", $cnt,
> did => __($did), count => $cnt);
> } else {
> print __nx("touched one path\n", "touched {count}
> paths\n", $cnt,
> count => $cnt);
> }
>
> Please correct me if I am wrong, and you know language where
> "added %d paths", "updated %d paths", "reverted %d paths" etc. must
> have
> different word order.
We may never know. :-) I prefer not to make assumptions about other
languages and I think there is not much to gain from using this
approach instead of marking entire sentence as the patch does. I mean
the code verbosity is almost the same but maybe it gets harder to
translate.
Other thing, we want to avoid marking for translation single words
(when that is avoidable) because those could appear on other sites that
need a different translation according to the context. For example:
'commit' could be a verb or a noun from the context.
> > When $cnt is 1 I get the following warning:
> > Redundant argument in printf at .../libexec/git-core/git-add
> > --interactive line 680.
>
> I wonder what is the case of C code - is similar warning here, or is
> gettext smarter in that case...
I do not know but I know there is a few cases like this in C code.
> > The singular form does not have a %d to consume $cnt argument to
> > printf(). Either we find a way to suppress that warning or we
> > change
> > the singular form to contain %d.
>
> Anyway, with __nx there should be no such problem.
>
> >
> >
> > >
> > > @@ -1508,8 +1514,10 @@ sub patch_update_file {
> > > ...
> > > - print colored
> > > $header_color, "Split into ",
> > > - scalar(@split), "
> > > hunks.\n";
> > > + print colored
> > > $header_color, sprintf(
> > > + Q__("Split into
> > > %d hunk.\n",
> > > + "Split into
> > > %d hunks.\n",
> > > + scalar(@spli
> > > t)), scalar(@split));
> >
> > Like we do with this.
>
> Note that it is a bit of change in behavior: previously Git would
> say "Split into 0 hunks.\n".
>
>
> Though this is probably more work that you wanted to do. The __n
> would
> take place of Q__, while the __nx function might be defined like this
> (borrowing from Locale::TextDomain), which needs to be put into
> Git::I18N:
>
> # Plural with interpolation.
> sub __nx ($$$@)
> {
> my ($msgid, $msgid_plural, $count, %args) = @_;
>
> return __expand ((__n $msgid, $msgid_plural, $count),
> %args);
> }
I thank you very much for your code suggestion and review but I am not
comfortable enough with perl to fiddle much more and add this to the
i18n work of the perl scripts, as I said before.
So if you find this a good think do add, please send patches as a
follow-up or to replace this patch.
^ permalink raw reply
* Re: [PATCH v2 02/11] i18n: add--interactive: mark simple here documents for translation
From: Vasco Almeida @ 2016-10-03 12:43 UTC (permalink / raw)
To: Jakub Narębski, Junio C Hamano
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
David Aguilar
In-Reply-To: <07371844-7fde-5b7f-b9e1-7db1a54fdbb5@gmail.com>
A Qui, 29-09-2016 às 23:27 +0200, Jakub Narębski escreveu:
> W dniu 29.09.2016 o 19:05, Junio C Hamano pisze:
> >
> > Vasco Almeida <vascomalmeida@sapo.pt> writes:
> >
> > >
> > > On the other hand, would it make sense to translate these
> > > commands? If
> > > so, we would mark for translation the commands name of @cmd in
> > > main_loop().
> > >
> > > sub main_loop {
> > > - my @cmd = ([ 'status', \&status_cmd, ],
> > > - [ 'update', \&update_cmd, ],
> > > - [ 'revert', \&revert_cmd, ],
> > > - [ 'add untracked', \&add_untracked_cmd, ],
> > > - [ 'patch', \&patch_update_cmd, ],
> > > - [ 'diff', \&diff_cmd, ],
> > > - [ 'quit', \&quit_cmd, ],
> > > - [ 'help', \&help_cmd, ],
> > > + my @cmd = ([ __('status'), \&status_cmd, ],
> > > + [ __('update'), \&update_cmd, ],
> > > + [ __('revert'), \&revert_cmd, ],
> > > + [ __('add untracked'), \&add_untracked_cmd, ],
> > > + [ __('patch'), \&patch_update_cmd, ],
> > > + [ __('diff'), \&diff_cmd, ],
> > > + [ __('quit'), \&quit_cmd, ],
> > > + [ __('help'), \&help_cmd, ],
> >
> > I don't know offhand. If the code to prompt and accept the command
> > given by the user can take the translated word (or a prefix of it),
> > theoretically I would say it could be made to work, but to me it is
> > dubious the benefit outweighs its downsides. It would make
> > teaching
> > Git and troubleshooting over the phone harder, I would guess.
> >
> > A: "Hi, I am in a 'git add -i' session."
> > B: "Give 's' at the prompt."
> > A: "My Git does not seem to take 's' as a valid command."
> > B: "What? I've never seen that problem."
> > ... back and forth wastes 10 minutes ...
> > A: "By the way, I am running Git in Portuguese."
>
> Also, for one-letter commands to work (there is setting where you
> don't even need to press enter, IIRC) all those translations would
> have to be chosen to begin with different letter, isn't it?
I choose not do mark those command names for translation in the next
re-roll since there is no obvious gain from it.
^ permalink raw reply
* Re: [PATCH v2 03/11] i18n: add--interactive: mark strings with interpolation for translation
From: Vasco Almeida @ 2016-10-03 12:41 UTC (permalink / raw)
To: Jakub Narębski, Junio C Hamano, git
Cc: Jiang Xin, Ævar Arnfjörð Bjarmason, David Aguilar
In-Reply-To: <b8c25ae3-143c-3e9b-0c7b-115a9b4756ae@gmail.com>
> W dniu 31.08.2016 o 14:31, Vasco Almeida pisze:
> > Use of sprintf following die or error_msg is necessary for
> > placeholder
> > substitution take place.
>
> No, it is not. Though I don't think that we have in out Git::I18N
> the support for Perl i18n placeholder substitution.
I will try to change the commit message to better reflect the reality.
> From gettext manual:
> https://www.gnu.org/software/gettext/manual/gettext.html#perl_002dfor
> mat
>
> 15.3.16 Perl Format Strings
>
> There are two kinds format strings in Perl: those acceptable to the
> Perl
> built-in function printf, labelled as ‘perl-format’, and those
> acceptable
> to the libintl-perl function __x, labelled as ‘perl-brace-format’.
>
> Perl printf format strings are described in the sprintf section of
> ‘man perlfunc’.
>
> Perl brace format strings are described in the
> Locale::TextDomain(3pm)
> manual page of the CPAN package libintl-perl. In brief, Perl format
> uses
> placeholders put between braces (‘{’ and ‘}’). The placeholder must
> have
> the syntax of simple identifiers.
>
> Git doesn't use Locale::TextDomain, from what I understand, to
> provide
> fallback in no-gettext case. Also, Locale::TextDomain is not in
> core.
Yes that can be a reason not to use Locale::TextDomain. When Ævar
Arnfjörð Bjarmason added gettext support and i18n stuff, he chose no to
use TextDomain because it did more than he wanted it to do, and that
could introduce bugs and unnecessary work.
5e9637c ("i18n: add infrastructure for translating Git with gettext",
2011-11-18)
https://public-inbox.org/git/AANLkTilYD_NyIZMyj9dHtVk-ylVBfvyxpCC7982LW
nVd@mail.gmail.com/
> > diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> > index e11a33d..4e1e857 100755
> > --- a/git-add--interactive.perl
> > +++ b/git-add--interactive.perl
> > @@ -612,12 +612,12 @@ sub list_and_choose {
> > else {
> > $bottom = $top = find_unique($choice, @stuff);
> > if (!defined $bottom) {
> > - error_msg "Huh ($choice)?\n";
> > + error_msg sprintf(__("Huh (%s)?\n"), $choice);
>
> So this would be, self explained without need of comment
> for translators:
>
> + error_msg __x ("Huh ({choice})?\n"), choice => $choice);
>
>
> > next TOPLOOP;
> > }
>
> Though this is probably more work that you wanted to do.
> The __x might be defined like this (borrowing from Locale::TextDomain),
> which needs to be put into perl/Git/I18N.pm
>
> sub __ ($);
> sub __expand ($%);
>
> # With interpolation.
> sub __x ($@)
> {
> my ($msgid, %vars) = @_;
>
> return __expand (__($msgid), %vars);
> }
>
> sub __expand ($%)
> {
> my ($translation, %args) = @_;
>
> my $re = join '|', map { quotemeta $_ } keys %args;
> $translation =~ s/\{($re)\}/defined $args{$1} ? $args{$1} : "{$1}"/ge;
>
> return $translation;
> }
I wonder if it is worth the trouble to add and use these functions,
when there is already a way that works and for me looks simpler. One
reason, if valid, would be that translators already translate strings
with %d and %s from C source which is where the majority of the English
text comes from. Thus it would make little difference for them.
If we use in perl string like in C there is a chance that there will be
a match of some string and would lead to just one msgid instead of two
in the git.pot template for translation. Actually this happens for the
string with "Huh (%s)?" in clean.c.
Unfortunately, I do not know if I would add these changes because I
know little about perl and hence I am not comfortable to do so.
Maybe if you see it is indeed worth adding these to Git I18N.pm, you
could send a follow-up patch or a replacement for this one.
^ permalink raw reply
* Re: [Q] would it be bad to make /etc/gitconfig runtime configurable?
From: Jeff King @ 2016-10-03 11:26 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.2.20.1610031303000.35196@virtualbox>
On Mon, Oct 03, 2016 at 01:06:10PM +0200, Johannes Schindelin wrote:
> Hi,
>
> On Tue, 27 Sep 2016, Jeff King wrote:
>
> > On Tue, Sep 27, 2016 at 10:05:37AM -0700, Junio C Hamano wrote:
> >
> > > The subject says it all. Would it be bad if we introduce an
> > > environment variable, GIT_SYSTEM_CONFIG=/etc/gitconfig, that names
> > > an alternative location of the system-wide configuration file?
> > >
> > > That would supersede/deprecate GIT_CONFIG_NOSYSTEM that we
> > > introduced primarily so that we can run our tests without getting
> > > affected by the configuration that happens to be effective on the
> > > host that the test is being run.
> >
> > I can't think of a reason it would be bad.
>
> I cannot think of any reason right now, either, but my gut tells me that
> this needs to simmer a while in the backs of our minds, to give potential
> reasons a chance to come forward.
>
> What would be the use case, BTW? IOW what would it solve that cannot
> already be solved by using XDG_CONFIG_HOME?
The patches Junio posted later use it for the test suite (and I also
have had to skip some tests in the past related to system config because
of its lack).
I would also use it when doing git experiments on GitHub servers. We
keep several relevant config settings in /etc/gitconfig, so if I were to
say, build a new version of git and test how it repacked torvalds/linux,
I need to make sure it picks up the same config. Usually I do it by
baking in the right /etc/gitconfig at build time, but it would be less
annoying to be able to override it at run-time.
I admit both of those are uses for git _developers_, though, not git
_users_.
-Peff
^ permalink raw reply
* Re: [Q] would it be bad to make /etc/gitconfig runtime configurable?
From: Johannes Schindelin @ 2016-10-03 11:06 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20160927192539.ybooc34iodnzs2ab@sigill.intra.peff.net>
Hi,
On Tue, 27 Sep 2016, Jeff King wrote:
> On Tue, Sep 27, 2016 at 10:05:37AM -0700, Junio C Hamano wrote:
>
> > The subject says it all. Would it be bad if we introduce an
> > environment variable, GIT_SYSTEM_CONFIG=/etc/gitconfig, that names
> > an alternative location of the system-wide configuration file?
> >
> > That would supersede/deprecate GIT_CONFIG_NOSYSTEM that we
> > introduced primarily so that we can run our tests without getting
> > affected by the configuration that happens to be effective on the
> > host that the test is being run.
>
> I can't think of a reason it would be bad.
I cannot think of any reason right now, either, but my gut tells me that
this needs to simmer a while in the backs of our minds, to give potential
reasons a chance to come forward.
What would be the use case, BTW? IOW what would it solve that cannot
already be solved by using XDG_CONFIG_HOME?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/3] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"
From: Duy Nguyen @ 2016-10-03 10:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqzimrj03j.fsf@gitster.mtv.corp.google.com>
On Thu, Sep 29, 2016 at 2:28 AM, Junio C Hamano <gitster@pobox.com> wrote:
> After reading the three patches through, however, I do not think we
> use the command line option anywhere. I'm inclined to say that we
> shouldn't add it at all. Or at least do so in a separate follow-up
> patch "now we have an internal mechanism, let's expose it anyway" at
> the end. Which means that the last sentence in my attempted rewrite
> should go.
We don't use it internally _yet_. I need to go through all the
external diff code and see --shift-ita should be there. The end goal
is still changing the default behavior and getting rid of --shift-ita,
after making sure we don't break stuff. I do use it though because
"git diff" is more often run in my workflow than "git status".
> As I already said, --shift-ita is not quite descriptive and I think
> it should be renamed to something else, but I kept that in the
> following attempt to rewrite:
It's meant to be a temporary thing (which could last a year or three,
depending on how fast I scan through the code base) so I didn't give
much thought on naming.
Umm... after a couple of minutes, I still couldn't think of any
better. The one-line summary of this change is "correct the position
of intent-to-add entries in diff", or as you put it more precisely
(with a bit paraphrasing), "make ita entries not exist in index". I
don't see any good way to shorten that to one or two words.
--ita-not-in-index good enough? Or maybe --[no-]ita-visible-in-index.
--
Duy
^ permalink raw reply
* Re: [PATCH 1/5] git-gui i18n: mark strings for translation
From: Pat Thoyts @ 2016-10-03 10:30 UTC (permalink / raw)
To: Vasco Almeida; +Cc: git, Shawn O . Pearce
In-Reply-To: <1462704778-4722-1-git-send-email-vascomalmeida@sapo.pt>
Vasco Almeida <vascomalmeida@sapo.pt> writes:
>Mark strings for translation in lib/index.tcl that were seemingly
>left behind by 700e560 ("git-gui: Mark forgotten strings for
>translation.", 2008-09-04) which marks string in do_revert_selection
>procedure.
>These strings are passed to unstage_help and add_helper procedures.
>
>Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
>---
> lib/index.tcl | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/lib/index.tcl b/lib/index.tcl
>index 74a81a7..3a3e534 100644
>--- a/lib/index.tcl
>+++ b/lib/index.tcl
>@@ -291,7 +291,7 @@ proc do_unstage_selection {} {
>
> if {[array size selected_paths] > 0} {
> unstage_helper \
>- {Unstaging selected files from commit} \
>+ [mc "Unstaging selected files from commit"] \
> [array names selected_paths]
> } elseif {$current_diff_path ne {}} {
> unstage_helper \
>@@ -343,7 +343,7 @@ proc do_add_selection {} {
>
> if {[array size selected_paths] > 0} {
> add_helper \
>- {Adding selected files} \
>+ [mc "Adding selected files"] \
> [array names selected_paths]
> } elseif {$current_diff_path ne {}} {
> add_helper \
>@@ -385,7 +385,7 @@ proc do_add_all {} {
> set paths [concat $paths $untracked_paths]
> }
> }
>- add_helper {Adding all changed files} $paths
>+ add_helper [mc "Adding all changed files"] $paths
> }
>
> proc revert_helper {txt paths} {
This series looks good. Especially noticing the misuse of the append
command. Applied to my pu for now.
Thank you,
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply
* Re: [PATCH 4/5] git-gui: fix incorrect use of Tcl append command
From: Pat Thoyts @ 2016-10-03 10:23 UTC (permalink / raw)
To: Vasco Almeida; +Cc: git, Shawn O . Pearce
In-Reply-To: <1462704778-4722-4-git-send-email-vascomalmeida@sapo.pt>
Vasco Almeida <vascomalmeida@sapo.pt> writes:
>Fix wrong use of append command in strings marked for translation.
>According to Tcl/Tk Documentation [1],
> append varName ?value value value ...?
>appends all value arguments to the current value of variable varName.
>This means that
> append "[appname] ([reponame]): " [mc "File Viewer"]
>is setting a variable named "[appname] ([reponame]): " to the output of
>[mc "File Viewer"], rather than returning the concatenation of both
>expressions as one might expect.
>
>The format for some strings enables, for instance, a French translator
>to translate like "%s (%s) : Create Branch" (space before colon).
>Conversely, strings already translated will be marked as fuzzy and the
>translator must update them herself.
>
>For some cases, use alternative way for concatenation instead of using
>strcat procedure defined in git-gui.sh.
>
>Reference: 31bb1d1 ("git-gui: Paper bag fix missing translated strings",
>2007-09-14) fixes the same issue slightly differently.
>
>[1] http://www.tcl.tk/man/tcl/TclCmd/append.htm
>
>Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
>---
> lib/blame.tcl | 2 +-
> lib/branch_checkout.tcl | 2 +-
> lib/branch_create.tcl | 2 +-
> lib/branch_delete.tcl | 2 +-
> lib/branch_rename.tcl | 2 +-
> lib/browser.tcl | 4 ++--
> lib/database.tcl | 2 +-
> lib/diff.tcl | 11 +++++------
> lib/error.tcl | 4 ++--
> lib/merge.tcl | 2 +-
> lib/remote_add.tcl | 2 +-
> lib/remote_branch_delete.tcl | 2 +-
> lib/shortcut.tcl | 6 +++---
> lib/tools_dlg.tcl | 6 +++---
> lib/transport.tcl | 2 +-
> 15 files changed, 25 insertions(+), 26 deletions(-)
>
>diff --git a/lib/blame.tcl b/lib/blame.tcl
>index b1d15f4..a1aeb8b 100644
>--- a/lib/blame.tcl
>+++ b/lib/blame.tcl
>@@ -70,7 +70,7 @@ constructor new {i_commit i_path i_jump} {
> set path $i_path
>
> make_toplevel top w
>- wm title $top [append "[appname] ([reponame]): " [mc "File Viewer"]]
>+ wm title $top [mc "%s (%s): File Viewer" [appname] [reponame]]
>
> set font_w [font measure font_diff "0"]
>
>diff --git a/lib/branch_checkout.tcl b/lib/branch_checkout.tcl
>index 2e459a8..d06037d 100644
>--- a/lib/branch_checkout.tcl
>+++ b/lib/branch_checkout.tcl
>@@ -13,7 +13,7 @@ constructor dialog {} {
> global use_ttk NS
> make_dialog top w
> wm withdraw $w
>- wm title $top [append "[appname] ([reponame]): " [mc "Checkout Branch"]]
>+ wm title $top [mc "%s (%s): Checkout Branch" [appname] [reponame]]
> if {$top ne {.}} {
> wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
> }
>diff --git a/lib/branch_create.tcl b/lib/branch_create.tcl
>index 4bb9077..ba367d5 100644
>--- a/lib/branch_create.tcl
>+++ b/lib/branch_create.tcl
>@@ -20,7 +20,7 @@ constructor dialog {} {
>
> make_dialog top w
> wm withdraw $w
>- wm title $top [append "[appname] ([reponame]): " [mc "Create Branch"]]
>+ wm title $top [mc "%s (%s): Create Branch" [appname] [reponame]]
> if {$top ne {.}} {
> wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
> }
>diff --git a/lib/branch_delete.tcl b/lib/branch_delete.tcl
>index 9aef0c9..a505163 100644
>--- a/lib/branch_delete.tcl
>+++ b/lib/branch_delete.tcl
>@@ -13,7 +13,7 @@ constructor dialog {} {
>
> make_dialog top w
> wm withdraw $w
>- wm title $top [append "[appname] ([reponame]): " [mc "Delete Branch"]]
>+ wm title $top [mc "%s (%s): Delete Branch" [appname] [reponame]]
> if {$top ne {.}} {
> wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
> }
>diff --git a/lib/branch_rename.tcl b/lib/branch_rename.tcl
>index 6e510ec..3a2d79a 100644
>--- a/lib/branch_rename.tcl
>+++ b/lib/branch_rename.tcl
>@@ -12,7 +12,7 @@ constructor dialog {} {
>
> make_dialog top w
> wm withdraw $w
>- wm title $top [append "[appname] ([reponame]): " [mc "Rename Branch"]]
>+ wm title $top [mc "%s (%s): Rename Branch" [appname] [reponame]]
> if {$top ne {.}} {
> wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
> }
>diff --git a/lib/browser.tcl b/lib/browser.tcl
>index 0328338..1580493 100644
>--- a/lib/browser.tcl
>+++ b/lib/browser.tcl
>@@ -24,7 +24,7 @@ constructor new {commit {path {}}} {
> global cursor_ptr M1B use_ttk NS
> make_dialog top w
> wm withdraw $top
>- wm title $top [append "[appname] ([reponame]): " [mc "File Browser"]]
>+ wm title $top [mc "%s (%s): File Browser" [appname] [reponame]]
>
> if {$path ne {}} {
> if {[string index $path end] ne {/}} {
>@@ -272,7 +272,7 @@ constructor dialog {} {
> global use_ttk NS
> make_dialog top w
> wm withdraw $top
>- wm title $top [append "[appname] ([reponame]): " [mc "Browse Branch Files"]]
>+ wm title $top [mc "%s (%s): Browse Branch Files" [appname] [reponame]]
> if {$top ne {.}} {
> wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
> wm transient $top .
>diff --git a/lib/database.tcl b/lib/database.tcl
>index 8bd4b8e..8578308 100644
>--- a/lib/database.tcl
>+++ b/lib/database.tcl
>@@ -63,7 +63,7 @@ proc do_stats {} {
> bind $w <Visibility> "grab $w; focus $w.buttons.close"
> bind $w <Key-Escape> [list destroy $w]
> bind $w <Key-Return> [list destroy $w]
>- wm title $w [append "[appname] ([reponame]): " [mc "Database Statistics"]]
>+ wm title $w [mc "%s (%s): Database Statistics" [appname] [reponame]]
> wm deiconify $w
> tkwait window $w
> }
>diff --git a/lib/diff.tcl b/lib/diff.tcl
>index 0d56986..1228ce2 100644
>--- a/lib/diff.tcl
>+++ b/lib/diff.tcl
>@@ -220,10 +220,9 @@ proc show_other_diff {path w m cont_info} {
> }
> $ui_diff conf -state normal
> if {$type eq {submodule}} {
>- $ui_diff insert end [append \
>- "* " \
>- [mc "Git Repository (subproject)"] \
>- "\n"] d_info
>+ $ui_diff insert end \
>+ "* [mc "Git Repository (subproject)"]\n" \
>+ d_info
> } elseif {![catch {set type [exec file $path]}]} {
> set n [string length $path]
> if {[string equal -length $n $path $type]} {
>@@ -608,7 +607,7 @@ proc apply_hunk {x y} {
> puts -nonewline $p $current_diff_header
> puts -nonewline $p [$ui_diff get $s_lno $e_lno]
> close $p} err]} {
>- error_popup [append $failed_msg "\n\n$err"]
>+ error_popup "$failed_msg\n\n$err"
> unlock_index
> return
> }
>@@ -826,7 +825,7 @@ proc apply_range_or_line {x y} {
> puts -nonewline $p $current_diff_header
> puts -nonewline $p $wholepatch
> close $p} err]} {
>- error_popup [append $failed_msg "\n\n$err"]
>+ error_popup "$failed_msg\n\n$err"
> }
>
> unlock_index
>diff --git a/lib/error.tcl b/lib/error.tcl
>index 9b7d229..71dc860 100644
>--- a/lib/error.tcl
>+++ b/lib/error.tcl
>@@ -17,7 +17,7 @@ proc error_popup {msg} {
> set cmd [list tk_messageBox \
> -icon error \
> -type ok \
>- -title [append "$title: " [mc "error"]] \
>+ -title [mc "%s: error" $title] \
> -message $msg]
> if {[winfo ismapped [_error_parent]]} {
> lappend cmd -parent [_error_parent]
>@@ -33,7 +33,7 @@ proc warn_popup {msg} {
> set cmd [list tk_messageBox \
> -icon warning \
> -type ok \
>- -title [append "$title: " [mc "warning"]] \
>+ -title [mc "%s: warning" $title] \
> -message $msg]
> if {[winfo ismapped [_error_parent]]} {
> lappend cmd -parent [_error_parent]
>diff --git a/lib/merge.tcl b/lib/merge.tcl
>index 460d32f..b2de109 100644
>--- a/lib/merge.tcl
>+++ b/lib/merge.tcl
>@@ -149,7 +149,7 @@ constructor dialog {} {
> }
>
> make_dialog top w
>- wm title $top [append "[appname] ([reponame]): " [mc "Merge"]]
>+ wm title $top [mc "%s (%s): Merge" [appname] [reponame]]
> if {$top ne {.}} {
> wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
> }
>diff --git a/lib/remote_add.tcl b/lib/remote_add.tcl
>index 50029d0..480a6b3 100644
>--- a/lib/remote_add.tcl
>+++ b/lib/remote_add.tcl
>@@ -17,7 +17,7 @@ constructor dialog {} {
>
> make_dialog top w
> wm withdraw $top
>- wm title $top [append "[appname] ([reponame]): " [mc "Add Remote"]]
>+ wm title $top [mc "%s (%s): Add Remote" [appname] [reponame]]
> if {$top ne {.}} {
> wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
> }
>diff --git a/lib/remote_branch_delete.tcl b/lib/remote_branch_delete.tcl
>index fcc06d0..5ba9fca 100644
>--- a/lib/remote_branch_delete.tcl
>+++ b/lib/remote_branch_delete.tcl
>@@ -26,7 +26,7 @@ constructor dialog {} {
> global all_remotes M1B use_ttk NS
>
> make_dialog top w
>- wm title $top [append "[appname] ([reponame]): " [mc "Delete Branch Remotely"]]
>+ wm title $top [mc "%s (%s): Delete Branch Remotely" [appname] [reponame]]
> if {$top ne {.}} {
> wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
> }
>diff --git a/lib/shortcut.tcl b/lib/shortcut.tcl
>index 78878ef..135086b 100644
>--- a/lib/shortcut.tcl
>+++ b/lib/shortcut.tcl
>@@ -5,7 +5,7 @@ proc do_windows_shortcut {} {
> global _gitworktree
> set fn [tk_getSaveFile \
> -parent . \
>- -title [append "[appname] ([reponame]): " [mc "Create Desktop Icon"]] \
>+ -title [mc "%s (%s): Create Desktop Icon" [appname] [reponame]] \
> -initialfile "Git [reponame].lnk"]
> if {$fn != {}} {
> if {[file extension $fn] ne {.lnk}} {
>@@ -37,7 +37,7 @@ proc do_cygwin_shortcut {} {
> }
> set fn [tk_getSaveFile \
> -parent . \
>- -title [append "[appname] ([reponame]): " [mc "Create Desktop Icon"]] \
>+ -title [mc "%s (%s): Create Desktop Icon" [appname] [reponame]] \
> -initialdir $desktop \
> -initialfile "Git [reponame].lnk"]
> if {$fn != {}} {
>@@ -69,7 +69,7 @@ proc do_macosx_app {} {
>
> set fn [tk_getSaveFile \
> -parent . \
>- -title [append "[appname] ([reponame]): " [mc "Create Desktop Icon"]] \
>+ -title [mc "%s (%s): Create Desktop Icon" [appname] [reponame]] \
> -initialdir [file join $env(HOME) Desktop] \
> -initialfile "Git [reponame].app"]
> if {$fn != {}} {
>diff --git a/lib/tools_dlg.tcl b/lib/tools_dlg.tcl
>index 7eeda9d..c05413c 100644
>--- a/lib/tools_dlg.tcl
>+++ b/lib/tools_dlg.tcl
>@@ -19,7 +19,7 @@ constructor dialog {} {
> global repo_config use_ttk NS
>
> make_dialog top w
>- wm title $top [append "[appname] ([reponame]): " [mc "Add Tool"]]
>+ wm title $top [mc "%s (%s): Add Tool" [appname] [reponame]]
> if {$top ne {.}} {
> wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
> wm transient $top .
>@@ -184,7 +184,7 @@ constructor dialog {} {
> load_config 1
>
> make_dialog top w
>- wm title $top [append "[appname] ([reponame]): " [mc "Remove Tool"]]
>+ wm title $top [mc "%s (%s): Remove Tool" [appname] [reponame]]
> if {$top ne {.}} {
> wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
> wm transient $top .
>@@ -280,7 +280,7 @@ constructor dialog {fullname} {
> }
>
> make_dialog top w -autodelete 0
>- wm title $top [append "[appname] ([reponame]): " $title]
>+ wm title $top "[mc "%s (%s):" [appname] [reponame]] $title"
> if {$top ne {.}} {
> wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
> wm transient $top .
>diff --git a/lib/transport.tcl b/lib/transport.tcl
>index e5d211e..a1a424a 100644
>--- a/lib/transport.tcl
>+++ b/lib/transport.tcl
>@@ -226,7 +226,7 @@ proc do_push_anywhere {} {
> bind $w <Visibility> "grab $w; focus $w.buttons.create"
> bind $w <Key-Escape> "destroy $w"
> bind $w <Key-Return> [list start_push_anywhere_action $w]
>- wm title $w [append "[appname] ([reponame]): " [mc "Push"]]
>+ wm title $w [mc "%s (%s): Push" [appname] [reponame]]
> wm deiconify $w
> tkwait window $w
> }
Well spotted!
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply
* Re: [PATCH 2/2] git-gui: ensure the file in the diff pane is in the list of selected files
From: Pat Thoyts @ 2016-10-03 10:02 UTC (permalink / raw)
To: Alex Riesen; +Cc: git, Junio C Hamano, Jakub Narębski, Johannes Schindelin
In-Reply-To: <20160628085925.GE3710@pflmari>
Alex Riesen <alexander.riesen@cetitec.com> writes:
>It is very confusing that the file which diff is displayed is marked as
>selected, but it is not in fact selected (that means the array of selected
>files does not include the file in question).
>
>Fixing this also improves the use of $FILENAMES in custom defined tools: one
>does not have to click the file in the list to make it selected.
>
>Signed-off-by: Alex Riesen <alexander.riesen@cetitec.com>
>---
> lib/diff.tcl | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/lib/diff.tcl b/lib/diff.tcl
>index 0d56986..30bdd69 100644
>--- a/lib/diff.tcl
>+++ b/lib/diff.tcl
>@@ -127,6 +127,9 @@ proc show_diff {path w {lno {}} {scroll_pos {}} {callback {}}} {
> } else {
> start_show_diff $cont_info
> }
>+
>+ global current_diff_path selected_paths
>+ set selected_paths($current_diff_path) 1
> }
>
> proc show_unmerged_diff {cont_info} {
These both look good. Applied with minor changes to the commit message
for patch 1.
Thank you,
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply
* Autoupdate / autoinit submodules?
From: Jeremy Morton @ 2016-10-03 9:50 UTC (permalink / raw)
To: git
Apologies if this has already been discussed, but did any of the
proposed patches to allow .gitmodules to be configured for a submodule
to be automatically recursive cloned and/or automatically updated on
pull make it in yet?
My use-case for submodules - which is shared libraries - almost always
requires a recursive clone, so having to remember the --recursive
param (or to use an alias) is annoying (and I usually forget), and it
usually requires pulling the latest master on a "git pull", too. I
think both of these things should be able to be automated through git
module configuration.
--
Best regards,
Jeremy Morton (Jez)
^ permalink raw reply
* Re: [PATCH] git-gui: stop using deprecated merge syntax
From: René Scharfe @ 2016-10-03 9:45 UTC (permalink / raw)
To: patthoyts, Stefan Beller
Cc: Junio C Hamano, Johannes Sixt, Git List, Dennis Kaarsemaker
In-Reply-To: <87mvilx2ak.fsf@red.patthoyts.tk>
Am 03.10.2016 um 10:30 schrieb Pat Thoyts:
> The only problem I see here is that generally git-gui tries to continue
> to work with older versions of git as well. So adding a guard using the
> git-version procedure should maintain that backwards compatibility.
Makes sense for a stand-alone tool.
> I suggest:
>
> From c2716458f05893ca88c05ce211a295a330e74590 Mon Sep 17 00:00:00 2001
> From: René Scharfe <l.s.r@web.de>
> Date: Sat, 24 Sep 2016 13:30:22 +0200
> Subject: [PATCH] git-gui: stop using deprecated merge syntax
>
> Starting with v2.5.0 git merge can handle FETCH_HEAD internally and
> warns when it's called like 'git merge <message> HEAD <commit>' because
> that syntax is deprecated. Use this feature in git-gui and get rid of
> that warning.
>
> Tested-by: Johannes Sixt <j6t@kdbg.org>
> Reviewed-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
OK, but perhaps move me from From: to Original-patch-by: as the version
check is a big enough change in itself. Or add a separate commit for
it. Or at least mention that you added the check in the commit message.
Thanks,
René
^ permalink raw reply
* Re: [PATCH v2 6/6] git-gui: Update Japanese information
From: Pat Thoyts @ 2016-10-03 8:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Satoshi Yasushima, git, Jakub Narębski
In-Reply-To: <xmqqk2enobol.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
>"Satoshi Yasushima" <s.yasushima@gmail.com> writes:
>
>> There seems to be a cause in the following taboo.
>> Sure, PATCH 3/6 is little too great.
>> http://vger.kernel.org/majordomo-info.html#taboo
>> >Taboo things to be done when discussing at VGER lists
>> : (abbr.)
>> > * Message size exceeding 100 000 characters causes blocking.
>>
>> To somehow.
>
>Since I received the patch directly bypassing vger, I queued it on
>gitgui-0.20.0 from Pat and tentatively merged it to my 'pu'.
>
>Pat, we haven't heard from you for a long time. How would you want
>to proceed? If you fetch from me and merge 52285c83 ("git-gui:
>update Japanese information", 2016-09-07), you'd get these six
>commits for po/ja.po and then we'd be in sync next time I pull from
>you.
>
>That is, if that ever happens--is git-gui pretty much in deep
>maintenance mode without anything more to do from your point of
>view?
>
>Thanks.
>
I'm just starting to catch up once again. hopefully I can be
a bit more reactive than recently. Merging 52285c83 looks fine. I'll
stick that onto the 0.20.0 head and see what else I can pick up on top.
There are a few from the git for windows set among others.
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply
* Re: [PATCH] git-gui: stop using deprecated merge syntax
From: Pat Thoyts @ 2016-10-03 8:30 UTC (permalink / raw)
To: Stefan Beller
Cc: Junio C Hamano, Johannes Sixt, René Scharfe, Git List,
Dennis Kaarsemaker
In-Reply-To: <CAGZ79kZZwum+97JGL8OFWTdODZeqrEyj4MVMC6o6ynFMOhiFLg@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>On Sun, Sep 25, 2016 at 11:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Johannes Sixt <j6t@kdbg.org> writes:
>>
>>> Am 24.09.2016 um 13:30 schrieb René Scharfe:
>>>> Starting with v2.5.0 git merge can handle FETCH_HEAD internally and
>>>> warns when it's called like 'git merge <message> HEAD <commit>' because
>>>> that syntax is deprecated. Use this feature in git-gui and get rid of
>>>> that warning.
>>>>
>>>> Signed-off-by: Rene Scharfe <l.s.r@web.de>
>>>> ---
>>>> Tested only _very_ lightly!
>>>>
>>>> git-gui/lib/merge.tcl | 7 +------
>>>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>>>
>>>> diff --git a/git-gui/lib/merge.tcl b/git-gui/lib/merge.tcl
>>>> index 460d32f..5ab6f8f 100644
>>>> --- a/git-gui/lib/merge.tcl
>>>> +++ b/git-gui/lib/merge.tcl
>>>> @@ -112,12 +112,7 @@ method _start {} {
>>>> close $fh
>>>> set _last_merged_branch $branch
>>>>
>>>> - set cmd [list git]
>>>> - lappend cmd merge
>>>> - lappend cmd --strategy=recursive
>>>> - lappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]]
>>>> - lappend cmd HEAD
>>>> - lappend cmd $name
>>>> + set cmd [list git merge --strategy=recursive FETCH_HEAD]
>>>>
>>>> ui_status [mc "Merging %s and %s..." $current_branch $stitle]
>>>> set cons [console::new [mc "Merge"] "merge $stitle"]
>>>>
>>>
>>> Much better than my version. I had left fmt-merge-msg and added
>>> --no-log to treat merge.log config suitably. But this works too, and
>>> is much more obvious.
>>>
>>> Tested-by: Johannes Sixt <j6t@kdbg.org>
>
>Reviewed-by: Stefan Beller <sbeller@google.com>
>
The only problem I see here is that generally git-gui tries to continue
to work with older versions of git as well. So adding a guard using the
git-version procedure should maintain that backwards compatibility.
I suggest:
From c2716458f05893ca88c05ce211a295a330e74590 Mon Sep 17 00:00:00 2001
From: René Scharfe <l.s.r@web.de>
Date: Sat, 24 Sep 2016 13:30:22 +0200
Subject: [PATCH] git-gui: stop using deprecated merge syntax
Starting with v2.5.0 git merge can handle FETCH_HEAD internally and
warns when it's called like 'git merge <message> HEAD <commit>' because
that syntax is deprecated. Use this feature in git-gui and get rid of
that warning.
Tested-by: Johannes Sixt <j6t@kdbg.org>
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
lib/merge.tcl | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/lib/merge.tcl b/lib/merge.tcl
index 460d32f..2361b78 100644
--- a/lib/merge.tcl
+++ b/lib/merge.tcl
@@ -112,13 +112,16 @@ method _start {} {
close $fh
set _last_merged_branch $branch
- set cmd [list git]
- lappend cmd merge
- lappend cmd --strategy=recursive
- lappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]]
- lappend cmd HEAD
- lappend cmd $name
-
+ if {[git-version >= "2.5.0"]} {
+ set cmd [list git merge --strategy=recursive FETCH_HEAD]
+ } else {
+ set cmd [list git]
+ lappend cmd merge
+ lappend cmd --strategy=recursive
+ lappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]]
+ lappend cmd HEAD
+ lappend cmd $name
+ }
ui_status [mc "Merging %s and %s..." $current_branch $stitle]
set cons [console::new [mc "Merge"] "merge $stitle"]
console::exec $cons $cmd [cb _finish $cons]
--
2.10.0.windows.1
^ permalink raw reply related
* Re: [PATCH 0/6] receive-pack: quarantine pushed objects
From: Christian Couder @ 2016-10-03 6:45 UTC (permalink / raw)
To: Jeff King; +Cc: git, David Turner
In-Reply-To: <20161002130216.bburwjkx4dkzxiig@sigill.intra.peff.net>
On Sun, Oct 2, 2016 at 3:02 PM, Jeff King <peff@peff.net> wrote:
> On Sun, Oct 02, 2016 at 11:20:59AM +0200, Christian Couder wrote:
>
>> I wonder if the patch you sent in:
>>
>> https://public-inbox.org/git/20160816144642.5ikkta4l5hyx6act@sigill.intra.peff.net/
>>
>> is still useful or not.
>
> It is potentially still useful for other code paths besides
> receive-pack. But if the main concern is pushes, then yeah, I think it
> is not really doing anything.
>
>> I guess if we fail the receive-pack because the pack is bigger than
>> receive.maxInputSize, then the "quarantine" directory will also be
>> removed, so the part of the pack that we received before failing the
>> receive-pack will be deleted.
>
> Correct. _Any_ failure up to the tmp_objdir_migrate() call will drop the
> objects. So that includes index-pack failing for any reason.
Great, thanks for explaining!
>> > These two patches set that up by letting index-pack and pre-receive
>> > know that quarantine path and use it to store arbitrary files that
>> > _don't_ get migrated to the main object database (i.e., the log file
>> > mentioned above).
>>
>> It would be nice to have a diffstat for the whole series.
>
> You mean in the cover letter? I do not mind including it if people find
> them useful, but I personally have always just found them to be clutter
> at that level.
I think it can help to quickly get an idea about what the series
impacts, and it would have made it easier for me to see that the
changes in the patch you sent previously
(https://public-inbox.org/git/20160816144642.5ikkta4l5hyx6act@sigill.intra.peff.net/)
are not part of this series.
Thanks anyway,
Christian.
^ permalink raw reply
* Feature Request: user defined suffix for temp files created by git-mergetool
From: Josef Ridky @ 2016-10-03 6:36 UTC (permalink / raw)
To: git
In-Reply-To: <88486231.114620.1475474318974.JavaMail.zimbra@redhat.com>
Hello,
I would like to request for implementing feature described in subject.
In several projects, we are using git mergetool for comparing files from different folders.
Unfortunately, when we have opened three files for comparing using meld tool (e.q. Old_version -- Result -- New_version),
we can see only name of temporary files created by mergetool in the labels (e.g. foo_REMOTE -- foo_BASE -- foo_LOCAL)
and users (and sometime even we) are confused, which of the files should they edit and save.
If you will be so kind and approve this feature request, you will help us to solve this unfortunate issue by the easiest way.
I have already prepared patch, which can be applied to resolve this request (see in attachment).
Best regards
Josef Ridky
Associate Software Engineer
Core Services Team
Red Hat Czech, s.r.o.
^ permalink raw reply
* Re: [PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2
From: René Scharfe @ 2016-10-02 22:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Git List, brian m. carlson
In-Reply-To: <xmqqwpiclu65.fsf@gitster.mtv.corp.google.com>
Am 15.09.2016 um 23:39 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>
>> Am 15.09.2016 um 22:01 schrieb Junio C Hamano:
>>> René Scharfe <l.s.r@web.de> writes:
>>>
>>>> Take this for example:
>>>>
>>>> - strbuf_addf(&o->obuf, _("(bad commit)\n"));
>>>> + strbuf_addstr(&o->obuf, _("(bad commit)\n"));
>>>>
>>>> If there's a language that uses percent signs instead of parens or as
>>>> regular letters, then they need to be escaped in the translated string
>>>> before, but not after the patch. As I wrote: silly.
>>>
>>> Ahh, OK, so "This use of addf only has format part and nothing else,
>>> hence the format part can be taken as-is" which is the Coccinelle rule
>>> used to produce this patch is incomplete and always needs manual
>>> inspection, in case the format part wanted to give a literal % in
>>> the output. E.g. it is a bug to convert this
>>>
>>> strbuf_addf(&buf, _("this is 100%% wrong!"));
>>>
>>> to
>>>
>>> strbuf_addstr(&buf, _("this is 100%% wrong!"));
>>
>> Right. Such strings seem to be quite rare in practice, though.
>>
>>> Thanks for clarification. Perhaps the strbuf.cocci rule file can
>>> have some comment to warn the person who builds *.patch file to look
>>> for % in E2, or something?
>>
>> Something like this?
>
> Yup, with something like that I would understdood where that
> puzzling question came from.
Here's something better than a comment:
-- >8 --
Subject: [PATCH] coccicheck: make transformation for strbuf_addf(sb, "...") more precise
We can replace strbuf_addf() calls that just add a simple string with
calls to strbuf_addstr() to make the intent clearer. We need to be
careful if that string contains printf format specifications like %%,
though, as a simple replacement would change the output.
Add checks to the semantic patch to make sure we only perform the
transformation if the second argument is a string constant (possibly
translated) that doesn't contain any percent signs.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
contrib/coccinelle/strbuf.cocci | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci
index 1e24298..63995f2 100644
--- a/contrib/coccinelle/strbuf.cocci
+++ b/contrib/coccinelle/strbuf.cocci
@@ -1,8 +1,31 @@
+@ strbuf_addf_with_format_only @
+expression E;
+constant fmt;
@@
-expression E1, E2;
+ strbuf_addf(E,
+(
+ fmt
+|
+ _(fmt)
+)
+ );
+
+@ script:python @
+fmt << strbuf_addf_with_format_only.fmt;
@@
-- strbuf_addf(E1, E2);
-+ strbuf_addstr(E1, E2);
+cocci.include_match("%" not in fmt)
+
+@ extends strbuf_addf_with_format_only @
+@@
+- strbuf_addf
++ strbuf_addstr
+ (E,
+(
+ fmt
+|
+ _(fmt)
+)
+ );
@@
expression E1, E2;
--
2.10.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox