* Re: git-scm.com status report
From: Jeff King @ 2017-02-03 22:24 UTC (permalink / raw)
To: pedro rijo; +Cc: Samuel Lijin, e, Git Users
In-Reply-To: <CAPMsMoAUcVteJGfyYrL1ZkNLnoRES0yZxkMZeL347Q_1Kx5VBQ@mail.gmail.com>
On Fri, Feb 03, 2017 at 09:23:33PM +0000, pedro rijo wrote:
> Seems a good idea. I will start by going through some old prs/issues to
> look for trash. If I do find some like the one I referred I will let you
> know by mentioning you. After that I will have a look at simpler issues/prs.
>
> Let me know if you do agree (or you recommend another workflow) so that I
> can start looking at it this weekend :)
That sounds perfect. Thanks!
-Peff
^ permalink raw reply
* [PATCH] commit: Optimize number of lstat() calls
From: Gumbel, Matthew K @ 2017-02-03 23:22 UTC (permalink / raw)
To: git@vger.kernel.org
When making a --only commit, original behavior was to do a full cache
update for the purposes of giving the pre-commit hook an up-to-date set
of stat data. That would result in long runtime for git-commit in a big
repo on NFS (>60s for a 54k-file repo).
With this change, when doing a --only commit and no pre-commit hook is
present, the cache update is skipped since it is known a priori which
files are to be committed.
This was discussed on the mailing list here:
https://public-inbox.org/git/DA0A42D68346B1469147552440A645039A9C56D4@ORSMSX115.amr.corp.intel.com/
Signed-off-by: Matthew K. Gumbel <matthew.k.gumbel@intel.com>
---
builtin/commit.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 2de5f6c..1df3d71 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -470,7 +470,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
add_remove_files(&partial);
- refresh_cache(REFRESH_QUIET);
+ if (find_hook("pre-commit")) {
+ refresh_cache(REFRESH_QUIET);
+ }
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
die(_("unable to write new_index file"));
@@ -482,7 +484,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
create_base_index(current_head);
add_remove_files(&partial);
- refresh_cache(REFRESH_QUIET);
+ if (find_hook("pre-commit")) {
+ refresh_cache(REFRESH_QUIET);
+ }
if (write_locked_index(&the_index, &false_lock, CLOSE_LOCK))
die(_("unable to write temporary index file"));
--
2.8.4
^ permalink raw reply related
* Re: [PATCH] commit: Optimize number of lstat() calls
From: Junio C Hamano @ 2017-02-03 23:38 UTC (permalink / raw)
To: Gumbel, Matthew K; +Cc: git@vger.kernel.org
In-Reply-To: <DA0A42D68346B1469147552440A645039A9C9988@ORSMSX115.amr.corp.intel.com>
"Gumbel, Matthew K" <matthew.k.gumbel@intel.com> writes:
> When making a --only commit, original behavior was to do a full cache
> update for the purposes of giving the pre-commit hook an up-to-date set
> of stat data. That would result in long runtime for git-commit in a big
> repo on NFS (>60s for a 54k-file repo).
>
> With this change, when doing a --only commit and no pre-commit hook is
> present, the cache update is skipped since it is known a priori which
> files are to be committed.
Did you determine that "pre-commit" hook is the only thing that
needs the special case and if so how? With that stated in the log
message, we would feel safe to take this patch.
> Signed-off-by: Matthew K. Gumbel <matthew.k.gumbel@intel.com>
> ---
> builtin/commit.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 2de5f6c..1df3d71 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -470,7 +470,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
>
> hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
> add_remove_files(&partial);
> - refresh_cache(REFRESH_QUIET);
> + if (find_hook("pre-commit")) {
> + refresh_cache(REFRESH_QUIET);
> + }
> update_main_cache_tree(WRITE_TREE_SILENT);
> if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
> die(_("unable to write new_index file"));
I wonder why this patch is full of 0xa0 bytes instead of spaces.
Somebody between your editor and mailing list is munging your
patches?
> @@ -482,7 +484,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
>
> create_base_index(current_head);
> add_remove_files(&partial);
> - refresh_cache(REFRESH_QUIET);
> + if (find_hook("pre-commit")) {
> + refresh_cache(REFRESH_QUIET);
> + }
>
> if (write_locked_index(&the_index, &false_lock, CLOSE_LOCK))
> die(_("unable to write temporary index file"));
I'd prefer if you introduced a small helper function, name it
can_cheat_on_refreshing() or something, and encapsulate the call to
find_hook("pre-commit") inside it---if there are other corner cases
you didn't think of, or if in the future more hooks are added that
need their incoming index file already refreshed, we do not want to
update these two places---rather we'd want to limit the need for
updating to the single helper function.
^ permalink raw reply
* Re: [PATCH] git-parse-remote.sh: Remove op_prep argument
From: Pranit Bauva @ 2017-02-04 0:04 UTC (permalink / raw)
To: Siddharth Kannan; +Cc: Git List
In-Reply-To: <1486146489-8877-1-git-send-email-kannan.siddharth12@gmail.com>
Hey Siddharth,
On Fri, Feb 3, 2017 at 11:58 PM, Siddharth Kannan
<kannan.siddharth12@gmail.com> wrote:
> - Remove the third argument of error_on_missing_default_upstream that is no
> longer required
> - FIXME to remove this argument was added in commit 045fac5845
This is not exactly correct. Well, this is the commit you get on
git-blame but it isn't really the one to look for. The "real" commit
when the variable was introduced was 15a147e61898 and was used for
writing out the error message. The commit 045fac5845 changed the error
message and the variable was not used then so it got redundant. So if
possible you could document all this information in the commit message
somehow, then it would be really great! :)
> - Run "grep" on the rest of the codebase to find and remove occurences of
/s/occurences/occurrences/g (spelling mistake ;))
> the third argument and fix the function calls appropriately
>
> Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
> ---
So if you want a better commit message then you could probably use this,
"parse-remote: remove reference to unused op_prep
This argument was introduced in the commit 15a147e618 to help in
writing out the error message but then in commit 045fac5845, the
reference to op_prep got removed. Thus the argument is no longer
useful and is removed.
"
> The contrib/examples/git-pull.sh file also has a variable op_prep which is used
> in one of the messages shown the user. Should I remove this variable as well?
Not really. We have kept the file git-pull.sh just as an example of
how git-pull was initially implemented. So previously git-pull was a
shell script which was then ported to C code. After that conversion,
the shell script was just put as it is in contrib/examples/ as a use
case of how git-pull should be implemented. I don't think there is any
need to modify it, but there isn't really a very strong reason to not
modify it (except that we don't usually write out the new changes to
it).
> contrib/examples/git-pull.sh | 2 +-
> git-parse-remote.sh | 3 +--
> git-rebase.sh | 2 +-
> 3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
> index 6b3a03f..1d51dc3 100755
> --- a/contrib/examples/git-pull.sh
> +++ b/contrib/examples/git-pull.sh
> @@ -267,7 +267,7 @@ error_on_no_merge_candidates () {
> echo "for your current branch, you must specify a branch on the command line."
> elif [ -z "$curr_branch" -o -z "$upstream" ]; then
> . git-parse-remote
> - error_on_missing_default_upstream "pull" $op_type $op_prep \
> + error_on_missing_default_upstream "pull" $op_type \
> "git pull <remote> <branch>"
> else
> echo "Your configuration specifies to $op_type $op_prep the ref '${upstream#refs/heads/}'"
> diff --git a/git-parse-remote.sh b/git-parse-remote.sh
> index d3c3998..9698a05 100644
> --- a/git-parse-remote.sh
> +++ b/git-parse-remote.sh
> @@ -56,8 +56,7 @@ get_remote_merge_branch () {
> error_on_missing_default_upstream () {
> cmd="$1"
> op_type="$2"
> - op_prep="$3" # FIXME: op_prep is no longer used
> - example="$4"
> + example="$3"
> branch_name=$(git symbolic-ref -q HEAD)
> display_branch_name="${branch_name#refs/heads/}"
> # If there's only one remote, use that in the suggestion
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 04f6e44..b89f960 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -448,7 +448,7 @@ then
> then
> . git-parse-remote
> error_on_missing_default_upstream "rebase" "rebase" \
> - "against" "git rebase $(gettext '<branch>')"
> + "git rebase $(gettext '<branch>')"
> fi
>
> test "$fork_point" = auto && fork_point=t
> --
> 2.1.4
>
>
^ permalink raw reply
* Re: [PATCH] document behavior of empty color name
From: Jeff King @ 2017-02-04 0:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, git
In-Reply-To: <xmqqy3xnf8jm.fsf@gitster.mtv.corp.google.com>
On Fri, Feb 03, 2017 at 10:12:13AM -0800, Junio C Hamano wrote:
> > Right, I think that's the correct behavior. The empty color name is a
> > real color ("none"), and you can put it in your list just like any other
> > color.
>
> Makes me wonder if we have a non-empty string that spells the same
> "do nothing", because ...
I don't think we do. "plain" or "none" or something would work. Or I
suppose "reset". That is not the same thing, but for practical purposes
it probably is.
> The above is just me "wondering"; I do not think what we have needs
> further tweaks--an empty after the final comma that means "do-nothing"
> is fine, I would think.
Yeah. I think doing anything more fancy opens more questions than it
answers.
-Peff
^ permalink raw reply
* Re: BUG: "git checkout -q -b foo origin/foo" is not quiet
From: Cornelius Weig @ 2017-02-04 0:55 UTC (permalink / raw)
To: Kevin Layer, git
In-Reply-To: <CAGSZTjLrdYJHixsUz0ha6=E263Z-vQuA11Oq=UNSVZfOmHkRuA@mail.gmail.com>
On 02/03/2017 10:36 PM, Kevin Layer wrote:
> Note that git version 1.8.3.1 is quiet and does not print the
> "tracking remote" message.
So what you are saying is, that git v1.8.3.1 *is* quiet, but v1.7.1 is
not? Sounds like a fixed bug to me.
I also checked with the latest version and it did not print anything
when used with -q.
You seem to urgently need quiet branch creation. Have you thought about
dumping unwanted output in the shell? E.g. in bash
$ git whatever >&/dev/null
^ permalink raw reply
* [PATCH] tag: add a config option for setting --annotate by default
From: David Aguilar @ 2017-02-04 2:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Make it easier for users to remember to annotate their tags.
Allow setting the default value for "--annotate" via the "tag.annotate"
configuration variable.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Documentation/config.txt | 5 +++++
builtin/tag.c | 15 ++++++++++++---
t/t7004-tag.sh | 23 +++++++++++++++++++++++
3 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index af2ae4cc02..0d562b97e9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2945,6 +2945,11 @@ submodule.alternateErrorStrategy
as computed via `submodule.alternateLocation`. Possible values are
`ignore`, `info`, `die`. Default is `die`.
+tag.annotate::
+ A boolean to specify whether annotated tags should be created by
+ default. If `--no-annotate` is specified on the command line,
+ it takes precedence over this option.
+
tag.forceSignAnnotated::
A boolean to specify whether annotated tags created should be GPG signed.
If `--annotate` is specified on the command line, it takes
diff --git a/builtin/tag.c b/builtin/tag.c
index 73df728114..1cf9bb73ad 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -29,6 +29,7 @@ static const char * const git_tag_usage[] = {
};
static unsigned int colopts;
+static int force_annotate;
static int force_sign_annotate;
static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
@@ -161,6 +162,10 @@ static int git_tag_config(const char *var, const char *value, void *cb)
status = git_gpg_config(var, value, cb);
if (status)
return status;
+ if (!strcmp(var, "tag.annotate")) {
+ force_annotate = git_config_bool(var, value);
+ return 0;
+ }
if (!strcmp(var, "tag.forcesignannotated")) {
force_sign_annotate = git_config_bool(var, value);
return 0;
@@ -326,7 +331,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct create_tag_options opt;
char *cleanup_arg = NULL;
int create_reflog = 0;
- int annotate = 0, force = 0;
+ int annotate = -1, force = 0;
int cmdmode = 0, create_tag_object = 0;
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
@@ -387,11 +392,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
opt.sign = 1;
set_signing_key(keyid);
}
- create_tag_object = (opt.sign || annotate || msg.given || msgfile);
if (argc == 0 && !cmdmode)
cmdmode = 'l';
+ if (force_annotate && !cmdmode && annotate == -1)
+ annotate = 1;
+
+ create_tag_object = (opt.sign || annotate > 0 || msg.given || msgfile);
+
if ((create_tag_object || force) && (cmdmode != 0))
usage_with_options(git_tag_usage, options);
@@ -478,7 +487,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die(_("Invalid cleanup mode %s"), cleanup_arg);
if (create_tag_object) {
- if (force_sign_annotate && !annotate)
+ if (force_sign_annotate && annotate == -1)
opt.sign = 1;
create_tag(object, tag, &buf, &opt, prev, object);
}
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 1cfa8a21d2..5ba52b57dd 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -754,6 +754,29 @@ echo from a fake editor.
EOF
chmod +x fakeeditor
+get_tag_header config-annotate $commit commit $time >expect
+./fakeeditor >>expect
+test_expect_success 'tag.annotate creates annotated tags' '
+ test_config tag.annotate true &&
+ GIT_EDITOR=./fakeeditor git tag config-annotate &&
+ get_tag_msg config-annotate >actual &&
+ test_cmp expect actual
+'
+test_expect_success 'tag --no-annotate overrides tag.annotate=true config' '
+ test_config tag.annotate true &&
+ GIT_EDITOR=false git tag --no-annotate cli-override-tag-annotate &&
+ tag_exists cli-override-tag-annotate
+'
+
+get_tag_header config-no-annotate $commit commit $time >expect
+./fakeeditor >>expect
+test_expect_success 'tag --annotate overrides tag.annotate=false config' '
+ test_config tag.annotate false &&
+ GIT_EDITOR=./fakeeditor git tag --annotate config-no-annotate &&
+ get_tag_msg config-no-annotate >actual &&
+ test_cmp expect actual
+'
+
get_tag_header implied-sign $commit commit $time >expect
./fakeeditor >>expect
echo '-----BEGIN PGP SIGNATURE-----' >>expect
--
2.11.0.486.gcc949b6e67.dirty
^ permalink raw reply related
* [PATCH] Remove --no-gui option from difftool usage string
From: Denton Liu @ 2017-02-04 2:56 UTC (permalink / raw)
To: git
The --no-gui option not documented in the manpage, nor is it actually
used in the source code. This change removes it from the usage help
that's printed.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
git-difftool.perl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index a5790d03a..657d5622d 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -29,8 +29,8 @@ sub usage
print << 'USAGE';
usage: git difftool [-t|--tool=<tool>] [--tool-help]
[-x|--extcmd=<cmd>]
- [-g|--gui] [--no-gui]
- [--prompt] [-y|--no-prompt]
+ [-g|--gui] [--prompt]
+ [-y|--no-prompt]
[-d|--dir-diff]
['git diff' options]
USAGE
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 00/12] completion: speed up refs completion
From: Jacob Keller @ 2017-02-04 3:15 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Junio C Hamano, Git mailing list
In-Reply-To: <CA+P7+xqhjVQMXtO4gtQ_2VAVd3qrpVLncH6YpFHreVV2mHORxg@mail.gmail.com>
On Thu, Feb 2, 2017 at 8:15 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Thu, Feb 2, 2017 at 6:53 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
>> This series speeds up refs completion for large number of refs, partly
>> by giving up disambiguating ambiguous refs (patch 6) and partly by
>> eliminating most of the shell processing between 'git for-each-ref'
>> and 'ls-remote' and Bash's completion facility. The rest is a bit of
>> preparatory reorganization, cleanup and bugfixes.
>>
>> The last patch touches the ZSH wrapper, too. By a lucky educated
>> guess I managed to get it work on the first try, but I don't really
>> know what I've actually done, so... ZSH users, please have a closer
>> look.
>>
>> At the end of this series refs completion from a local repository is
>> as fast as it can possibly get, at least as far as the completion
>> script is concerned, because it basically does nothing anymore :) All
>> it does is run 'git for-each-ref' with assorted options to do all the
>> work, and feed its output directly, without any processing into Bash's
>> COMPREPLY array. There is still room for improvements in the code
>> paths using 'git ls-remote', but for that we would need enhancements
>> to 'ls-remote'.
>>
>> It goes on top of the __gitdir() improvements series I just posted at:
>>
>> http://public-inbox.org/git/20170203024829.8071-1-szeder.dev@gmail.com/T/
>>
>> This series is also available at:
>>
>> https://github.com/szeder/git completion-refs-speedup
>>
>
> Nice! This is something i've been bothered by in the past since
> completion would take a rather long time!
>
> Regards,
> Jake
I haven't had a chance to further investigate, but I tried this series
out (from your github) and it appears that this series (or the
previous series for __gitdir work) breaks "git log" ref completion.
I'll have further details when I am able to investigate a it more.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH] tag: add a config option for setting --annotate by default
From: Junio C Hamano @ 2017-02-04 5:02 UTC (permalink / raw)
To: David Aguilar; +Cc: git
In-Reply-To: <20170204021402.15927-1-davvid@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> Make it easier for users to remember to annotate their tags.
> Allow setting the default value for "--annotate" via the "tag.annotate"
> configuration variable.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
I do not care too strongly about this, but I need to point out that
this will have fallout to tools and scripts. E.g. if you have this
configured and try to create a new tag in gitk, wouldn't this part
if {$msg != {}} {
exec git tag -a -m $msg $tag $id
} else {
exec git tag $tag $id
}
try to open an editor somehow to get the message even when $msg is
an empty string? I think the same problem already exists for the
tag.forceSignAnnotated variable we already have added, though.
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index af2ae4cc02..0d562b97e9 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2945,6 +2945,11 @@ submodule.alternateErrorStrategy
> as computed via `submodule.alternateLocation`. Possible values are
> `ignore`, `info`, `die`. Default is `die`.
>
> +tag.annotate::
> + A boolean to specify whether annotated tags should be created by
> + default. If `--no-annotate` is specified on the command line,
> + it takes precedence over this option.
> +
> tag.forceSignAnnotated::
> A boolean to specify whether annotated tags created should be GPG signed.
> If `--annotate` is specified on the command line, it takes
^ permalink raw reply
* Re: [PATCH] git-parse-remote.sh: Remove op_prep argument
From: Junio C Hamano @ 2017-02-04 5:09 UTC (permalink / raw)
To: Pranit Bauva; +Cc: Siddharth Kannan, Git List
In-Reply-To: <CAFZEwPMGTzVuLMSzm8wiNxvia4AV0T79C1ZTfcuO4=Bydz_zQA@mail.gmail.com>
Pranit Bauva <pranit.bauva@gmail.com> writes:
> So if you want a better commit message then you could probably use this,
> "parse-remote: remove reference to unused op_prep
>
> This argument was introduced in the commit 15a147e618 to help in
> writing out the error message but then in commit 045fac5845, the
> reference to op_prep got removed. Thus the argument is no longer
> useful and is removed.
> "
Expand the reference to commits like so:
15a147e618 ("rebase: use @{upstream} if no upstream specified",
2011-02-09)
Also pay attention to the subject, in which it is unclear whose
argument "op_prep" is. Other than that, your rewrite is more
readable than the original.
The error_on_missing_default_upstream helper function learned to
take op_prep argument with 15a147e618 ("rebase: use @{upstream}
if no upstream specified", 2011-02-09), but as of 045fac5845
("i18n: git-parse-remote.sh: mark strings for translation",
2016-04-19), the argument no longer is used. Remove it.
>> The contrib/examples/git-pull.sh file also has a variable op_prep which is used
>> in one of the messages shown the user. Should I remove this variable as well?
>
> Not really. We have kept the file git-pull.sh just as an example of
> how git-pull was initially implemented. So previously git-pull was a
> shell script which was then ported to C code. After that conversion,
> the shell script was just put as it is in contrib/examples/ as a use
> case of how git-pull should be implemented.
Yes, with s/should/could/. I agree with you that we should leave it
as-is.
^ permalink raw reply
* [L10N] Kickoff of translation for Git 2.12.0 round 1
From: Jiang Xin @ 2017-02-04 5:27 UTC (permalink / raw)
To: Alexander Shopov, Jordi Mas, Ralf Thielow, Jean-Noël Avila,
Marco Paolone, Changwoo Ryu, Vasco Almeida, Dimitriy Ryazantcev,
Peter Krefting, Trần Ngọc Quân, Jiang Xin,
Ray Chen
Cc: Git List, Junio C Hamano
Hi,
Git v2.12.0-rc0 has been released, and it's time to start new round of git l10n.
This time there are 200+ updated messages need to be translated since last
update:
l10n: git.pot: v2.12.0 round 1 (239 new, 15 removed)
Generate po/git.pot from v2.12.0-rc0 for git v2.12.0 l10n round 1.
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
You can get it from the usual place:
https://github.com/git-l10n/git-po/
As how to update your XX.po and help to translate Git, please see
"Updating a XX.po file" and other sections in “po/README" file.
--
Jiang Xin
^ permalink raw reply
* Re: [PATCH] Remove --no-gui option from difftool usage string
From: Jacob Keller @ 2017-02-04 5:58 UTC (permalink / raw)
To: Denton Liu; +Cc: Git mailing list
In-Reply-To: <20170204025617.GA9221@arch-attack.localdomain>
On Fri, Feb 3, 2017 at 6:56 PM, Denton Liu <liu.denton@gmail.com> wrote:
> The --no-gui option not documented in the manpage, nor is it actually
> used in the source code. This change removes it from the usage help
> that's printed.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> git-difftool.perl | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-difftool.perl b/git-difftool.perl
> index a5790d03a..657d5622d 100755
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -29,8 +29,8 @@ sub usage
> print << 'USAGE';
> usage: git difftool [-t|--tool=<tool>] [--tool-help]
> [-x|--extcmd=<cmd>]
> - [-g|--gui] [--no-gui]
> - [--prompt] [-y|--no-prompt]
> + [-g|--gui] [--prompt]
> + [-y|--no-prompt]
> [-d|--dir-diff]
> ['git diff' options]
> USAGE
> --
> 2.11.0
>
Aren't "--no-foo" options automatically created for booleans when
using our option parsing code?
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH 00/12] completion: speed up refs completion
From: Junio C Hamano @ 2017-02-04 6:21 UTC (permalink / raw)
To: Jacob Keller; +Cc: SZEDER Gábor, Git mailing list
In-Reply-To: <CA+P7+xpeyebN3pmSX09Avh1EvYVjALpBCQ9r2==q3SWTu3GMSw@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
> On Thu, Feb 2, 2017 at 8:15 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> On Thu, Feb 2, 2017 at 6:53 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
>>> This series speeds up refs completion for large number of refs, partly
>>> by giving up disambiguating ambiguous refs (patch 6) and partly by
>>> ...
>>> It goes on top of the __gitdir() improvements series I just posted at:
>>>
>>> http://public-inbox.org/git/20170203024829.8071-1-szeder.dev@gmail.com/T/
>>>
>> Nice! This is something i've been bothered by in the past since
>> completion would take a rather long time!
>
> I haven't had a chance to further investigate, but I tried this series
> out (from your github) and it appears that this series (or the
> previous series for __gitdir work) breaks "git log" ref completion.
> I'll have further details when I am able to investigate a it more.
Thanks, both. I'll look forward to how the story unfolds from
sidelines ;-)
^ permalink raw reply
* Re: [PATCH] Remove --no-gui option from difftool usage string
From: Denton Liu @ 2017-02-04 6:23 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list
In-Reply-To: <CA+P7+xoXWP=Cuqw3Pa1sFCChiw6wbNTEpvNm3WDuBHQPc_OjnA@mail.gmail.com>
On Fri, Feb 03, 2017 at 09:58:09PM -0800, Jacob Keller wrote:
> On Fri, Feb 3, 2017 at 6:56 PM, Denton Liu <liu.denton@gmail.com> wrote:
> > The --no-gui option not documented in the manpage, nor is it actually
> > used in the source code. This change removes it from the usage help
> > that's printed.
> >
> > Signed-off-by: Denton Liu <liu.denton@gmail.com>
> > ---
> > git-difftool.perl | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/git-difftool.perl b/git-difftool.perl
> > index a5790d03a..657d5622d 100755
> > --- a/git-difftool.perl
> > +++ b/git-difftool.perl
> > @@ -29,8 +29,8 @@ sub usage
> > print << 'USAGE';
> > usage: git difftool [-t|--tool=<tool>] [--tool-help]
> > [-x|--extcmd=<cmd>]
> > - [-g|--gui] [--no-gui]
> > - [--prompt] [-y|--no-prompt]
> > + [-g|--gui] [--prompt]
> > + [-y|--no-prompt]
> > [-d|--dir-diff]
> > ['git diff' options]
> > USAGE
> > --
> > 2.11.0
> >
>
> Aren't "--no-foo" options automatically created for booleans when
> using our option parsing code?
>
> Thanks,
> Jake
Sorry, I guess I didn't notice that. Would it make sense to document it
in the manpage then?
--
Denton Liu
^ permalink raw reply
* [PATCH] Add --gui option to mergetool
From: Denton Liu @ 2017-02-04 6:43 UTC (permalink / raw)
To: git
* fix the discrepancy between difftool and mergetool where
the former has the --gui flag and the latter does not by adding the
functionality to mergetool
* make difftool read 'merge.guitool' as a fallback, in accordance to the
manpage for difftool: "git difftool falls back to git mergetool
config variables when the difftool equivalents have not been defined"
* add guitool-related completions
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
Documentation/git-difftool.txt | 3 ++-
Documentation/git-mergetool.txt | 8 +++++++-
contrib/completion/git-completion.bash | 6 ++++--
git-mergetool.sh | 5 ++++-
4 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index 224fb3090..0b5d29237 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -89,7 +89,8 @@ instead. `--no-symlinks` is the default on Windows.
--gui::
When 'git-difftool' is invoked with the `-g` or `--gui` option
the default diff tool will be read from the configured
- `diff.guitool` variable instead of `diff.tool`.
+ `diff.guitool` variable instead of `diff.tool`. If `diff.guitool`
+ is not defined, it will try and read from `merge.guitool`.
--[no-]trust-exit-code::
'git-difftool' invokes a diff tool individually on each file.
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 3622d6648..2ab56efcf 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -8,7 +8,7 @@ git-mergetool - Run merge conflict resolution tools to resolve merge conflicts
SYNOPSIS
--------
[verse]
-'git mergetool' [--tool=<tool>] [-y | --[no-]prompt] [<file>...]
+'git mergetool' [--tool=<tool>] [-g|--gui] [-y | --[no-]prompt] [<file>...]
DESCRIPTION
-----------
@@ -64,6 +64,12 @@ variable `mergetool.<tool>.trustExitCode` can be set to `true`.
Otherwise, 'git mergetool' will prompt the user to indicate the
success of the resolution after the custom tool has exited.
+-g::
+--gui::
+ When 'git-mergetool' is invoked with the `-g` or `--gui` option
+ the default diff tool will be read from the configured
+ `merge.guitool` variable instead of `merge.tool`.
+
--tool-help::
Print a list of merge tools that may be used with `--tool`.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf8d..8a7427f3c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1268,7 +1268,7 @@ _git_difftool ()
--base --ours --theirs
--no-renames --diff-filter= --find-copies-harder
--relative --ignore-submodules
- --tool="
+ --tool= --gui"
return
;;
esac
@@ -1566,7 +1566,7 @@ _git_mergetool ()
return
;;
--*)
- __gitcomp "--tool="
+ __gitcomp "--tool= --gui"
return
;;
esac
@@ -2189,6 +2189,7 @@ _git_config ()
diff.submodule
diff.suppressBlankEmpty
diff.tool
+ diff.guitool
diff.wordRegex
diff.algorithm
difftool.
@@ -2290,6 +2291,7 @@ _git_config ()
merge.renormalize
merge.stat
merge.tool
+ merge.guitool
merge.verbosity
mergetool.
mergetool.keepBackup
diff --git a/git-mergetool.sh b/git-mergetool.sh
index e52b4e4f2..a17668752 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O<orderfile>] [file to merge] ...'
+USAGE='[--tool=tool] [-g|--gui] [--tool-help] [-y|--no-prompt|--prompt] [-O<orderfile>] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
@@ -414,6 +414,9 @@ main () {
shift ;;
esac
;;
+ -g|--gui)
+ merge_tool=$(git config merge.guitool)
+ ;;
-y|--no-prompt)
prompt=false
;;
--
2.11.0.21.ga274e0a.dirty
^ permalink raw reply related
* Re: [PATCH] commit: Optimize number of lstat() calls
From: Junio C Hamano @ 2017-02-04 6:50 UTC (permalink / raw)
To: Gumbel, Matthew K; +Cc: git@vger.kernel.org
In-Reply-To: <DA0A42D68346B1469147552440A645039A9C9988@ORSMSX115.amr.corp.intel.com>
Aside from whitespace breakage, I think we should never skip the
refreshing of the real index that is left after "git commit"
finishes.
Between these two calls to refresh_cache(), the one that writes out
a temporary index that contains the contents of HEAD plus the
contents of the working tree for the specified paths may be fine
without refreshing, unless somebody else (like the pre-commit hook)
looks at it. But the other one refreshes the real index file that
will be used after "git commit" returns the control. Users and
scripts that run "git commit" inside expect that the entries in the
resulting index are refreshed after "git commit" returns, and I do
not think of a safe way to optimizing it out; unlike the other one,
to which we can say "as long as there is no pre-commit hook, nobody
will look at it after we are done", there does not an easy-to-check
set of conditions that we can use to decide when it is safe to skip
refreshing.
Besides, leaving the main index not refreshed would mean the user
has to pay the refreshing cost when s/he runs other commands "git
diff", "git status", etc. after "git commit" for the first time;
so...
^ permalink raw reply
* Re: [PATCH] commit: Optimize number of lstat() calls
From: Johannes Schindelin @ 2017-02-04 0:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Gumbel, Matthew K, git@vger.kernel.org
In-Reply-To: <xmqqwpd678lx.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 3 Feb 2017, Junio C Hamano wrote:
> Aside from whitespace breakage, I think we should never skip the
> refreshing of the real index that is left after "git commit"
> finishes.
>
> Between these two calls to refresh_cache(), the one that writes out
> a temporary index that contains the contents of HEAD plus the
> contents of the working tree for the specified paths may be fine
> without refreshing, unless somebody else (like the pre-commit hook)
> looks at it. But the other one refreshes the real index file that
> will be used after "git commit" returns the control. Users and
> scripts that run "git commit" inside expect that the entries in the
> resulting index are refreshed after "git commit" returns, and I do
> not think of a safe way to optimizing it out; unlike the other one,
> to which we can say "as long as there is no pre-commit hook, nobody
> will look at it after we are done", there does not an easy-to-check
> set of conditions that we can use to decide when it is safe to skip
> refreshing.
>
> Besides, leaving the main index not refreshed would mean the user
> has to pay the refreshing cost when s/he runs other commands "git
> diff", "git status", etc. after "git commit" for the first time;
> so...
I am not sure... when you run `git diff` and `git status`, the index is
refreshed *anyway*, so with the patch under discussion we would save one
round of lstat() calls, for the same effect.
I could imagine that there is a third option we should consider, too: only
lstat() and update the paths that match the pathspec(s) provided on the
command line (this is the semantic meaning of the --only option, after
all: "I am only interested in these paths as far as this commit is
concerned"). What do you think?
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] commit: Optimize number of lstat() calls
From: Junio C Hamano @ 2017-02-04 7:23 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Gumbel, Matthew K, git@vger.kernel.org
In-Reply-To: <alpine.DEB.2.20.1702040129430.3496@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Besides, leaving the main index not refreshed would mean the user
>> has to pay the refreshing cost when s/he runs other commands "git
>> diff", "git status", etc. after "git commit" for the first time;
>> so...
>
> I am not sure... when you run `git diff` and `git status`, the index is
> refreshed *anyway*, so with the patch under discussion we would save one
> round of lstat() calls, for the same effect.
Yeah, you're right. The only ones that could be affected are
plumbing commands, and scripts that use plumbing are expected to be
written without relying on the "refreshed"-ness of the index they
are given (iow, if they want to rely on, they are expected to refresh
first before using plumbing commands). So there is no downside of
leaving the index in an unrefreshed state as long as everbody plays
by the rule.
> I could imagine that there is a third option we should consider, too: only
> lstat() and update the paths that match the pathspec(s) provided on the
> command line (this is the semantic meaning of the --only option, after
> all: "I am only interested in these paths as far as this commit is
> concerned"). What do you think?
I wondered that myself when I read the first message from Matthew
and noticed that we always refresh the entire index. But if it is
OK to leave the entire index un-refreshed, that would even be
simpler ;-)
^ permalink raw reply
* How I review patches
From: Junio C Hamano @ 2017-02-04 7:27 UTC (permalink / raw)
To: git
Here is a write-up on how I review patches posted on the list, in
the hope that knowing what to expect may help contributors [*0*].
But before going into exactly how I do my reviews, here is a short
list of the goals of doing reviews. I review a patch (or a set of
patches) to ensure that including it to our codebase will NOT:
* hurt users of existing versions of Git. [*1*]
* hurt users of the version of Git the change appears in. [*2*]
* make life harder for developers [*3*] of Git in the future. [*4*]
The remaining paragraphs of this message are not to be taken as me
telling other reviewers how they must conduct their reviews. I will
queue, and I have queued, patches reviewed by other reviewers
without doing detailed reviews myself, as long as I trust that the
reviewers share the same goals in their reviews, and as long as I
trust their competence and taste. How they exactly review may be
different from how I would have reviewed the patches, and that is
perfectly fine.
1. Description of the problem being solved.
When I see a patch (or a set of patches), I first read the proposed
log message, documentation update and new in-code comments.
These are places where the contributor can (and is expected to)
explain the motivation behind the change. I read them to make sure
that they clearly state what problem is being solved, why a
particular solution was chosen, what exactly that solution is, and
what other solutions were considered but discarded.
Just like "X is broken" in a bug report is not clear enough, "Fix X"
is often not enough in the proposed log message, as it is not clear
which part of what X does is wrong in the contributor's mind, and
why the contributor thinks it is broken. Saying "X currently does Y
but it should do Z instead to help such and such use case." would
help reviewers (and future developers who will read it) understand
the motivation behind the change.
A new feature or enhancement that is worth adding by default needs
to be explained how that new thing works and why it is there to the
end users, so a lack of documentation update is noticed at this
stage as well.
When I find that the explanation is lacking after reading the cover
letter, proposed log message, and documentation updates, I often ask
the contributor to elaborate, before going into the actual diff. I
often suggest "perhaps you meant this?", and I end up reading the
actual diff to base my best guess on in order to do so. This is
also where I notice a tricky code whose "why" is under-explained in
in-code comments [*5*].
To such review comments, I do not want the contributor to just say
"yes, you now understand what I wanted to do with this change". I
want to see the log message, in-code comments and documentation
updated so that other reviewers and future developers will not have
to ask the same question as I asked again.
2. Design of the solution.
After clarifying the original motivation of the contributor, it
sometimes becomes apparent that the patch aims too low and attempts
to solve too narrow an issue. I would point out that, within the
context of the patches, they can and should solve a wider range of
problems of the same class [*6*]. Or the patch may hurt users in
use cases that the contributor did not consider, and the solution
may need to be designed to cover these cases [*7*]. This design
review may cause us to iterate until we have a good description of
the problem and design of the solution.
3. "Code" review.
Once we made sure that the motivation is made clear, the scope of
the change is refined, and the design of the solution described
clearly, I dive into the code changes. What I look for primarily
during this phase is to see what the code does matches the desired
behaviour we established before this phase for correctness. This is
what many people think of as "code review", and it ranges from
spotting style issues and typoes, finding and fixing stupid
off-by-one errors, to noticing future maintainance issues caused by
using a wrong abstration or a misdesigned API.
During the review of the actual code change, I may discover that
some common corner cases are not handled properly, which I would
point out. Or the contributor may have thought about tricky corner
cases and handled them correctly in the patches, but did not explain
the "what" and "why" in the log message. Recording what cases were
considered and decided based on what reasoning in the log message is
important to help future developers and sets the course of evolution
of the codebase, and this may result in updates in the "explanation
of the changes" reviewed early in the cycle.
It is not like I never look at the code until log message and
explanation is perfect; to reduce the number of back-and-forth, I do
comment on the code even before it becomes clear if the design is
sound and clearly described. But at the conceptual level, because
the motivation guides the design and the design guides the
implementation, I tend to review the patches in this order.
Hope the above helps current and future contributors when they are
preparing and reviewing their patch series before submitting.
[Footnotes]
*0* If disclosing this to contributors turns out to be a good idea,
we may want to add a polished version of this to somewhere in
Documentation/ next to SubmittingPatches.
*1* For example, we need to be very careful when changing the
on-disk or on-wire data. We as developers may always run the
bleeding-edge version of Git, but how well do users with older
version of Git interact with our new shiny toys? Backward
compatibility issues can hurt users of existing versions Git
that do not have the change the patch introduces.
*2* That's called a "bug" in general, but can take different forms
(i.e. implementation bug, documentation bug, design bug, etc.).
Maybe the documentation promises to (or it can be misread to
promise to) do A when the actual code does B instead. Maybe the
code does A most of the time but silently does something else in
a corner case, without documenting it. Maybe the problem the
patches wanted to solve have two different ways to solve,
and the patches chose one way to solve and implemented it
correctly as it documented the feature, but the approach taken
may be a way that forces users to use Git in an awkward way or
encourages them to employ a bad workflow, when there is another
way that helps users better.
*3* Future users of Git cannot be hurt more than the patches
themselves hurts them immediately. Patches may make a design
mistake that makes it hard or impossible to extend a part of the
system further, and users can be robbed their opportunity to use
even better Git in the future---but the "even better Git" does
not exist yet when the patches are accepted anyway, and their
user experience at least does not get worse, at least. The
developers will find a way to work it around and transition
existing users to allow such an extension into existence and
their work may be made harder by such a design mistake, though.
*4* A set of patches may add a helper function that is useful for any
NUL-terminated string, but may make the helper take a pointer to
a strbuf because the callers to it in the patches happen to all
have the string in a strbuf. That forces future developers who
want to call the helper to either wrap their string in a strbuf
or to fix the misdesigned API to the helper function to take a
simple "char *".
A set of patches may introduce a new helper function to split
fields in a string whose format is well known throughout the
system, for which there already exists a helper function to do
the splitting. This doubles the amount of work required when
the format of the string needs to be extended in the future.
A set of patches may conflate two semantically distinct sets of
things and try to parse elements of both set with a single
helper function, only because the elements in these two sets
happen to be the same in the version immediately after applying
the patches. This forces future developers who need to add
more elements to one set without affecting others to split the
helper into two.
All of the above make it harder for developers that need to
enhance the system in the future, but they will NOT hurt users
of existing version or the version immediately after the patches
land. Such a patch series may happen to work correctly
at that version, and no amount of tests or field trials will
reveal these maintainance issues.
*5* In-code comments that do not explain why the code does things in
a particular way but just translates what it does from C to
English add to maintenance burden of having to keep it in sync
with the code, without adding much value to the readers of the
code. They instead should explain why the code does what it
does.
*6* For example, a contributor may originally wants to solve
something only for tags, but it is not uncommon that the issue
that exists for tags are shared by other kinds of refs, and it
may be better to solve it uniformly across tags, branches, etc.
*7* "cover"ing other uses cases does not necessarily have to be done
perfectly. In less likely situations, it may be OK to punt and
say "my code cannot handle it" and die() with a message, and
that is much much better than not considering these situations
and doing wrong things.
^ permalink raw reply
* [PATCH v2] parse-remote: Remove reference to unused op_prep
From: Siddharth Kannan @ 2017-02-04 8:00 UTC (permalink / raw)
To: git; +Cc: pranit.bauva, Siddharth Kannan
The error_on_missing_default_upstream helper function learned to
take op_prep argument with 15a147e618 ("rebase: use @{upstream}
if no upstream specified", 2011-02-09), but as of 045fac5845
("i18n: git-parse-remote.sh: mark strings for translation",
2016-04-19), the argument is no longer used. Remove it.
Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
---
Thanks a lot, Pranit and Junio for your reviews on the first version of this
patch. I have changed the messages accordingly.
contrib/examples/git-pull.sh | 2 +-
git-parse-remote.sh | 3 +--
git-rebase.sh | 2 +-
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
index 6b3a03f..1d51dc3 100755
--- a/contrib/examples/git-pull.sh
+++ b/contrib/examples/git-pull.sh
@@ -267,7 +267,7 @@ error_on_no_merge_candidates () {
echo "for your current branch, you must specify a branch on the command line."
elif [ -z "$curr_branch" -o -z "$upstream" ]; then
. git-parse-remote
- error_on_missing_default_upstream "pull" $op_type $op_prep \
+ error_on_missing_default_upstream "pull" $op_type \
"git pull <remote> <branch>"
else
echo "Your configuration specifies to $op_type $op_prep the ref '${upstream#refs/heads/}'"
diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index d3c3998..9698a05 100644
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -56,8 +56,7 @@ get_remote_merge_branch () {
error_on_missing_default_upstream () {
cmd="$1"
op_type="$2"
- op_prep="$3" # FIXME: op_prep is no longer used
- example="$4"
+ example="$3"
branch_name=$(git symbolic-ref -q HEAD)
display_branch_name="${branch_name#refs/heads/}"
# If there's only one remote, use that in the suggestion
diff --git a/git-rebase.sh b/git-rebase.sh
index 04f6e44..b89f960 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -448,7 +448,7 @@ then
then
. git-parse-remote
error_on_missing_default_upstream "rebase" "rebase" \
- "against" "git rebase $(gettext '<branch>')"
+ "git rebase $(gettext '<branch>')"
fi
test "$fork_point" = auto && fork_point=t
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] commit: Optimize number of lstat() calls
From: Johannes Schindelin @ 2017-02-04 10:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Gumbel, Matthew K, git@vger.kernel.org
In-Reply-To: <xmqqd1ey5shc.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 3 Feb 2017, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > I could imagine that there is a third option we should consider, too: only
> > lstat() and update the paths that match the pathspec(s) provided on the
> > command line (this is the semantic meaning of the --only option, after
> > all: "I am only interested in these paths as far as this commit is
> > concerned"). What do you think?
>
> I wondered that myself when I read the first message from Matthew
> and noticed that we always refresh the entire index. But if it is
> OK to leave the entire index un-refreshed, that would even be
> simpler ;-)
Hah! You're right, that would be much simpler ;-)
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v2] reset: add an example of how to split a commit into two
From: Duy Nguyen @ 2017-02-04 11:06 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git Mailing List, Jacob Keller
In-Reply-To: <20170203202833.17666-1-jacob.e.keller@intel.com>
On Sat, Feb 4, 2017 at 3:28 AM, Jacob Keller <jacob.e.keller@intel.com> wrote:
> +------------
> +$ git reset HEAD^ <1>
It may be a good idea to add -N here, so that 'add -p' can pick up the
new files if they are added in HEAD.
> +$ git add -p <2>
--
Duy
^ permalink raw reply
* Re: [PATCH v2] parse-remote: Remove reference to unused op_prep
From: Pranit Bauva @ 2017-02-04 11:25 UTC (permalink / raw)
To: Siddharth Kannan; +Cc: Git List
In-Reply-To: <1486195204-26901-1-git-send-email-kannan.siddharth12@gmail.com>
Hey SIddharth,
> Subject: parse-remote: Remove reference to unused op_prep
^
Minor nit: after the colon, we generally don't use the word starting
with an uppercase letter which I think can be figured out when you run
`git log -p git-parse-remote.sh`
On Sat, Feb 4, 2017 at 1:30 PM, Siddharth Kannan
<kannan.siddharth12@gmail.com> wrote:
> The error_on_missing_default_upstream helper function learned to
> take op_prep argument with 15a147e618 ("rebase: use @{upstream}
> if no upstream specified", 2011-02-09), but as of 045fac5845
> ("i18n: git-parse-remote.sh: mark strings for translation",
> 2016-04-19), the argument is no longer used. Remove it.
>
> Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
> ---
> Thanks a lot, Pranit and Junio for your reviews on the first version of this
> patch. I have changed the messages accordingly.
>
> contrib/examples/git-pull.sh | 2 +-
> git-parse-remote.sh | 3 +--
> git-rebase.sh | 2 +-
> 3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
> index 6b3a03f..1d51dc3 100755
> --- a/contrib/examples/git-pull.sh
> +++ b/contrib/examples/git-pull.sh
> @@ -267,7 +267,7 @@ error_on_no_merge_candidates () {
> echo "for your current branch, you must specify a branch on the command line."
> elif [ -z "$curr_branch" -o -z "$upstream" ]; then
> . git-parse-remote
> - error_on_missing_default_upstream "pull" $op_type $op_prep \
> + error_on_missing_default_upstream "pull" $op_type \
> "git pull <remote> <branch>"
> else
> echo "Your configuration specifies to $op_type $op_prep the ref '${upstream#refs/heads/}'"
I guess I suggested you to not change the file in contrib/ in my
earlier email[1] and to which Junio agreed too[2]. Is there any
confusion?
> diff --git a/git-parse-remote.sh b/git-parse-remote.sh
> index d3c3998..9698a05 100644
> --- a/git-parse-remote.sh
> +++ b/git-parse-remote.sh
> @@ -56,8 +56,7 @@ get_remote_merge_branch () {
> error_on_missing_default_upstream () {
> cmd="$1"
> op_type="$2"
> - op_prep="$3" # FIXME: op_prep is no longer used
> - example="$4"
> + example="$3"
> branch_name=$(git symbolic-ref -q HEAD)
> display_branch_name="${branch_name#refs/heads/}"
> # If there's only one remote, use that in the suggestion
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 04f6e44..b89f960 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -448,7 +448,7 @@ then
> then
> . git-parse-remote
> error_on_missing_default_upstream "rebase" "rebase" \
> - "against" "git rebase $(gettext '<branch>')"
> + "git rebase $(gettext '<branch>')"
> fi
>
> test "$fork_point" = auto && fork_point=t
> --
> 2.1.4
>
[1]: http://public-inbox.org/git/CAFZEwPMGTzVuLMSzm8wiNxvia4AV0T79C1ZTfcuO4=Bydz_zQA@mail.gmail.com/
[2]: http://public-inbox.org/git/xmqqd1ey8rul.fsf@gitster.mtv.corp.google.com/
^ permalink raw reply
* Re: [PATCH v2] parse-remote: Remove reference to unused op_prep
From: siddharth @ 2017-02-04 11:36 UTC (permalink / raw)
To: Pranit Bauva; +Cc: Git List
In-Reply-To: <CAFZEwPPyJBD0a+Jkr7EBCurFzvHLvZY9r_SFWS2wyW7QmmP4pg@mail.gmail.com>
On Sat, Feb 04, 2017 at 04:55:45PM +0530, Pranit Bauva wrote:
> Hey SIddharth,
>
> > Subject: parse-remote: Remove reference to unused op_prep
> ^
>
> Minor nit: after the colon, we generally don't use the word starting
> with an uppercase letter which I think can be figured out when you run
> `git log -p git-parse-remote.sh`
Oh, I am really sorry to have missed this. I will fix this and send a
third version of this patch.
>
> On Sat, Feb 4, 2017 at 1:30 PM, Siddharth Kannan
> <kannan.siddharth12@gmail.com> wrote:
> > The error_on_missing_default_upstream helper function learned to
> > take op_prep argument with 15a147e618 ("rebase: use @{upstream}
> > if no upstream specified", 2011-02-09), but as of 045fac5845
> > ("i18n: git-parse-remote.sh: mark strings for translation",
> > 2016-04-19), the argument is no longer used. Remove it.
> >
> > Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
> > ---
> > Thanks a lot, Pranit and Junio for your reviews on the first version of this
> > patch. I have changed the messages accordingly.
> >
> > contrib/examples/git-pull.sh | 2 +-
> > git-parse-remote.sh | 3 +--
> > git-rebase.sh | 2 +-
> > 3 files changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
> > index 6b3a03f..1d51dc3 100755
> > --- a/contrib/examples/git-pull.sh
> > +++ b/contrib/examples/git-pull.sh
> > @@ -267,7 +267,7 @@ error_on_no_merge_candidates () {
> > echo "for your current branch, you must specify a branch on the command line."
> > elif [ -z "$curr_branch" -o -z "$upstream" ]; then
> > . git-parse-remote
> > - error_on_missing_default_upstream "pull" $op_type $op_prep \
> > + error_on_missing_default_upstream "pull" $op_type \
> > "git pull <remote> <branch>"
> > else
> > echo "Your configuration specifies to $op_type $op_prep the ref '${upstream#refs/heads/}'"
>
> I guess I suggested you to not change the file in contrib/ in my
> earlier email[1] and to which Junio agreed too[2]. Is there any
> confusion?
Oh, you want me to completely remove the contrib/examples/ change
because that's the old shell implementation. Got it, I just checked
the log for that file and realised that it hasn't been changed
in a long time.
>
> > diff --git a/git-parse-remote.sh b/git-parse-remote.sh
> > index d3c3998..9698a05 100644
> > --- a/git-parse-remote.sh
> > +++ b/git-parse-remote.sh
> > @@ -56,8 +56,7 @@ get_remote_merge_branch () {
> > error_on_missing_default_upstream () {
> > cmd="$1"
> > op_type="$2"
> > - op_prep="$3" # FIXME: op_prep is no longer used
> > - example="$4"
> > + example="$3"
> > branch_name=$(git symbolic-ref -q HEAD)
> > display_branch_name="${branch_name#refs/heads/}"
> > # If there's only one remote, use that in the suggestion
> > diff --git a/git-rebase.sh b/git-rebase.sh
> > index 04f6e44..b89f960 100755
> > --- a/git-rebase.sh
> > +++ b/git-rebase.sh
> > @@ -448,7 +448,7 @@ then
> > then
> > . git-parse-remote
> > error_on_missing_default_upstream "rebase" "rebase" \
> > - "against" "git rebase $(gettext '<branch>')"
> > + "git rebase $(gettext '<branch>')"
> > fi
> >
> > test "$fork_point" = auto && fork_point=t
> > --
> > 2.1.4
> >
>
> [1]: http://public-inbox.org/git/CAFZEwPMGTzVuLMSzm8wiNxvia4AV0T79C1ZTfcuO4=Bydz_zQA@mail.gmail.com/
> [2]: http://public-inbox.org/git/xmqqd1ey8rul.fsf@gitster.mtv.corp.google.com/
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox