* Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard
From: Jeff King @ 2017-01-21 14:03 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <CACsJy8DxCPCXUeEoR1pQBCNqYOHjpgfLy_vgqsK1H=C0Mf=L8g@mail.gmail.com>
On Sat, Jan 21, 2017 at 07:44:05PM +0700, Duy Nguyen wrote:
> You just gave me a reason to rebuild urxvt with perl support. It
> solves my problem with SHA-1 nicely (and fixes another problem with
> very large counter in my approach, when you scroll git-log further
> down, because I can't know what's on screen or already scrolled past).
Note that even without my module, you can probably use the "matcher"
extension which is included with urxvt to do something similar (it can
handle arbitrary numbers of matching regexps).
The things I like about my module (and why I bothered to write it) are:
- it does nothing at all until you trigger it, so you don't have to
worry about the cost of your regexes, or how ugly false positives
will look when underlined
- I think the hint mode is much better for keyboard navigation.
The matcher extension does have a "list" mode where you get an
overlay with the whole list and select by number. That's what I
started with in mine, but I found it's really hard to use when there
are a lot of matches. Especially with sha1s.
> Do you think it should be in contrib for better discoverability (than
> burying it in the mail archive)? A separate git repo is also ok, but I
> guess that's just more burden.
I'm not sure it makes sense in git's contrib. It's really more about
urxvt than it is about git.
-Peff
^ permalink raw reply
* [RFC] what content should go in https://git-scm.com/doc/ext
From: Jeff King @ 2017-01-21 13:55 UTC (permalink / raw)
To: git
I'm wondering if anybody has opinions on:
https://github.com/git/git-scm.com/pull/924
(and I suspect most people in this community do not read pull requests
there, hence this post).
Basically, we maintain a list of links to outside documentation, as well
as to books. Somebody has requested a link to their paid tutorial
course. How should we handle it?
If the resource is valuable, then it may make sense to link to it, even
if it costs money. We already do this with book links, and my policy has
been to link any relevant book that is requested (I don't read them for
quality, so I have no opinions).
Should we do the same for tutorial content like this?
-Peff
^ permalink raw reply
* Re: [PATCH] git-p4: Fix git-p4.mapUser on Windows
From: Lars Schneider @ 2017-01-21 13:32 UTC (permalink / raw)
To: George Vanburgh; +Cc: Git mailing list, Luke Diamand
In-Reply-To: <01020159c0e82598-e373cf0d-2bad-41bb-b455-6896ad183e22-000000@eu-west-1.amazonses.com>
> On 21 Jan 2017, at 13:02, George Vanburgh <george@vanburgh.me> wrote:
>
> From: George Vanburgh <gvanburgh@bloomberg.net>
>
> When running git-p4 on Windows, with multiple git-p4.mapUser entries in
> git config - no user mappings are applied to the generated repository.
>
> Reproduction Steps:
>
> 1. Add multiple git-p4.mapUser entries to git config on a Windows
> machine
> 2. Attempt to clone a p4 repository
>
> None of the user mappings will be applied.
>
> This issue is caused by the fact that gitConfigList,
> uses split(os.linesep) to convert the output of
> git config --get-all into a list.
>
> On Windows, os.linesep is equal to '\r\n' - however git.exe
> returns configuration with a line seperator of '\n'. This leads to
> the list returned by gitConfigList containing only one element - which
> contains the full output of git config --get-all in string form. This
> causes problems for the code introduced to getUserMapFromPerforceServer
> in 10d08a1.
>
> This issue should be caught by the test introduced in 10d08a1, and
> would require running on Windows to reproduce. When running inside
> MinGW/Cygwin, however, os.linesep correctly returns '\n', and everything
> works as expected.
This surprises me. I would expect `\r\n` in a MinGW env...
Nevertheless, I wouldn't have caught that as I don't run the git-p4 tests
on Windows...
> The simplest fix for this issue would be to convert the line split logic
> inside gitConfigList to use splitlines(), which splits on any standard
> line delimiter. However, this function was only introduced in Python
> 2.7, and would mean a bump in the minimum required version of Python
> required to run git-p4. The alternative fix, implemented here, is to use
> '\n' as a delimiter, which git.exe appears to output consistently on
> Windows anyway.
Well, that also means if we ever use splitlines() then your fix below
would brake the code, right?
Python 2.7 was released 7 years ago in 2010. Therefore, I would vote to
bump the minimum version. But that's just my opinion :-)
> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
> ---
> git-p4.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index f427bf6..c134a58 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -656,7 +656,7 @@ def gitConfigInt(key):
> def gitConfigList(key):
> if not _gitConfig.has_key(key):
> s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
> - _gitConfig[key] = s.strip().split(os.linesep)
> + _gitConfig[key] = s.strip().split("\n")
I can't easily reproduce this as I don't have a running git-p4 setup on Windows.
However, your explanation and your fix make sense to me. If we don't want to bump
the version then this looks good to me.
Cheers,
Lars
^ permalink raw reply
* [RFC PATCH] Option to allow cherry-pick to skip empty commits
From: Giuseppe Bilotta @ 2017-01-21 13:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Giuseppe Bilotta
This allows cherry-picking a set of commits, some of which may be
redundant, without stopping to ask for the user intervention.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
Documentation/git-cherry-pick.txt | 4 ++++
builtin/revert.c | 1 +
sequencer.c | 45 +++++++++++++++++++++++++++++++--------
sequencer.h | 1 +
4 files changed, 42 insertions(+), 9 deletions(-)
I would like to add to cherry-pick the ability to skip patches. To this
end, I'm working on two options: a general `--skip-empty` option to
handle redundant and empty commits by simply skipping them (no user
intervention), and a `--skip` option as an alternative form to
`--continue` to skip the ongoing (conflicting or empty) commit.
The patch here presents my implementation of the `--skip-empty` option,
including documentation. Comments welcome.
However, it has been some time since I dwelved into git internals and
I'm having issues with the `--skip` option. My idea was to rely on the
existing implementation for `--continue`, and have it run the rollback
instead of the commit. However, I'm finding that after the rollback is
done (I'm using the existing rollback function in sequencer, which calls
`git reset --merge`, but I've also tried a `reset --hard`) the sequencer
does not continue because e.g. if the cherry-pick had stopped due to a
conflict, it still finds unmerged paths, and I can't find a way to tell
the sequencer to just re-read the index from scratch. Opinions welcome
(I can provide my WIP patch to make discussion easier).
On the same note, I've noticed that `git reset` clears CHERRY_PICK_HEAD.
I find this a little confusing, both for humans for the code itself
(since it doesn't really abort an ongoing cherry-pick, yet the sequencer
will think there is no running cherry-pick). This is particularly bad
because `git reset` is one of the strategies proposed to solve conflicts
in cherry-picking. So I was wondering if this was intentional or a side
effect of something else that I might look into cleaning up during this
patch series.
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index d35d771fc8..ffced816d6 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -138,6 +138,10 @@ effect to your index in a row.
examine the commit. This option overrides that behavior and
creates an empty commit object. Implies `--allow-empty`.
+--skip-empty::
+ This option causes empty and redundant cherry-picked commits to
+ be skipped without requesting the user intervention.
+
--strategy=<strategy>::
Use the given merge strategy. Should only be used once.
See the MERGE STRATEGIES section in linkgit:git-merge[1]
diff --git a/builtin/revert.c b/builtin/revert.c
index 4ca5b51544..ffdd367f99 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -102,6 +102,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
OPT_BOOL(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
OPT_BOOL(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
+ OPT_BOOL(0, "skip-empty", &opts->skip_empty, N_("skip redundant, empty commits")),
OPT_END(),
};
options = parse_options_concat(options, cp_extra);
diff --git a/sequencer.c b/sequencer.c
index 9adb7bbf1d..72962fd2fa 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -550,22 +550,32 @@ static int is_original_commit_empty(struct commit *commit)
/*
* Do we run "git commit" with "--allow-empty"?
+ *
+ * Or do we just skip this empty commit?
+ *
+ * Returns 1 if a commit should be done with --allow-empty,
+ * 0 if a commit should be done without --allow-empty,
+ * 2 if no commit should be done at all (skip empty commit)
+ * negative values in case of error
+ *
*/
-static int allow_empty(struct replay_opts *opts, struct commit *commit)
+static int allow_or_skip_empty(struct replay_opts *opts, struct commit *commit)
{
int index_unchanged, empty_commit;
/*
- * Three cases:
+ * Four cases:
*
- * (1) we do not allow empty at all and error out.
+ * (1) we do not allow empty at all and error out;
*
- * (2) we allow ones that were initially empty, but
+ * (2) we skip empty commits altogether;
+ *
+ * (3) we allow ones that were initially empty, but
* forbid the ones that become empty;
*
- * (3) we allow both.
+ * (4) we allow both.
*/
- if (!opts->allow_empty)
+ if (!opts->allow_empty && !opts->skip_empty)
return 0; /* let "git commit" barf as necessary */
index_unchanged = is_index_unchanged();
@@ -574,6 +584,9 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
if (!index_unchanged)
return 0; /* we do not have to say --allow-empty */
+ if (opts->skip_empty)
+ return 2;
+
if (opts->keep_redundant_commits)
return 1;
@@ -612,7 +625,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
const char *base_label, *next_label;
struct commit_message msg = { NULL, NULL, NULL, NULL };
struct strbuf msgbuf = STRBUF_INIT;
- int res, unborn = 0, allow;
+ int res = 0, unborn = 0, allow;
if (opts->no_commit) {
/*
@@ -771,12 +784,13 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
goto leave;
}
- allow = allow_empty(opts, commit);
+ allow = allow_or_skip_empty(opts, commit);
if (allow < 0) {
res = allow;
goto leave;
}
- if (!opts->no_commit)
+ /* allow == 2 means skip this commit */
+ if (allow != 2 && !opts->no_commit)
res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(),
opts, allow, opts->edit, 0, 0);
@@ -983,6 +997,8 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
opts->signoff = git_config_bool_or_int(key, value, &error_flag);
else if (!strcmp(key, "options.record-origin"))
opts->record_origin = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.skip-empty"))
+ opts->skip_empty = git_config_bool_or_int(key, value, &error_flag);
else if (!strcmp(key, "options.allow-ff"))
opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
else if (!strcmp(key, "options.mainline"))
@@ -1231,6 +1247,8 @@ static int save_opts(struct replay_opts *opts)
res |= git_config_set_in_file_gently(opts_file, "options.signoff", "true");
if (opts->record_origin)
res |= git_config_set_in_file_gently(opts_file, "options.record-origin", "true");
+ if (opts->skip_empty)
+ res |= git_config_set_in_file_gently(opts_file, "options.skip-empty", "true");
if (opts->allow_ff)
res |= git_config_set_in_file_gently(opts_file, "options.allow-ff", "true");
if (opts->mainline) {
@@ -1306,6 +1324,14 @@ int sequencer_continue(struct replay_opts *opts)
if ((res = read_populate_todo(&todo_list, opts)))
goto release_todo_list;
+ /* check if there is something to commit */
+ res = is_index_unchanged();
+ if (res < 0)
+ goto release_todo_list;
+
+ if (res && opts->skip_empty)
+ goto skip_this_commit;
+
/* Verify that the conflict has been resolved */
if (file_exists(git_path_cherry_pick_head()) ||
file_exists(git_path_revert_head())) {
@@ -1317,6 +1343,7 @@ int sequencer_continue(struct replay_opts *opts)
res = error_dirty_index(opts);
goto release_todo_list;
}
+skip_this_commit:
todo_list.current++;
res = pick_commits(&todo_list, opts);
release_todo_list:
diff --git a/sequencer.h b/sequencer.h
index 7a513c576b..c747cfcfc7 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -23,6 +23,7 @@ struct replay_opts {
int allow_empty;
int allow_empty_message;
int keep_redundant_commits;
+ int skip_empty;
int mainline;
--
2.11.0.585.g56041942c3.dirty
^ permalink raw reply related
* Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.
From: Duy Nguyen @ 2017-01-21 12:55 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Beller, Git Mailing List
In-Reply-To: <20170120191728.l3ne5tt5pwbmafjh@sigill.intra.peff.net>
On Sat, Jan 21, 2017 at 2:17 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Jan 19, 2017 at 11:30:23AM -0800, Stefan Beller wrote:
>
>> Now let's ask the same question for "git -C sub status ." (which is a
>> command that is only reading and not writing to the repository)
>>
>> 1) If the submodule is populated, the user clearly intended to know
>> more about the submodules status
>> 2) It is unclear if the user wanted to learn about the submodules state
>> (So ideally: "The submodule 'sub' is not initialized. To init ...")
>> or the status check should be applied to the superproject instead.
>>
>> Avoid the confusion in 2) as well and just error out for now. Later on
>> we may want to add another flag to git.c to allow commands to be run
>> inside unpopulated submodules and each command reacts appropriately.
>
> I like the general idea of catching commands in unpopulated submodules,
> but I'm somewhat uncomfortable with putting an unconditional check into
> git.c, for two reasons:
>
> 1. Reading the index can be expensive. You would not want "git
> rev-parse" to incur this cost.
>
> 2. How does this interact with commands which do interact with the
> index? Don't they expect to find the_index unpopulated?
>
> (I notice that it's effectively tied to RUN_SETUP, which is good.
> But that also means that many commands, like "diff", won't get the
> benefit. Not to mention non-builtins).
>
> I'd rather see it in the commands themselves. Especially given the
> "ideal" in your status example, which requires command-specific
> knowledge.
I agree. It's already bad enough for pathspec code to peek into the
index, adding a hidden dependency between parse_pathspec() and
read_cache(). And I still think parse_pathspec() is not the right
place to check submodule paths. Worktree should be checked as well, in
the case that the submodule is not yet registered in the index. The
right place to do that is per-command, with their consent so to speak,
because they may need to set things up (index, .git/config and stuff)
properly before explicitly doing this check.
--
Duy
^ permalink raw reply
* Re: [PATCH] log: new option decorate reflog of remote refs
From: Duy Nguyen @ 2017-01-21 12:48 UTC (permalink / raw)
To: Jacob Keller, Jeff King; +Cc: Git Mailing List
In-Reply-To: <CA+P7+xqgyUW-Wt2oUSCc86HG-MfL-itAu2yVrZ219LNwqQnwKw@mail.gmail.com>
On Sat, Jan 21, 2017 at 5:00 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Fri, Jan 20, 2017 at 6:30 AM, Jeff King <peff@peff.net> wrote:
>>> Imposing order between options could cause confusion, I think, if you
>>> remove --decorate-reflog leaving --remotes on by accident, now you get
>>> --remotes with a new meaning. We could go with something like
>>> --decodate-reflog=remote, but that clashes with the number of reflog
>>> entries and we may need a separator, like --decorate-reflog=remote,3.
>>> Or we could add something to --decorate= in addition to
>>> short|full|auto|no. Something like --decorate=full,reflog or
>>> --decorate=full,reflog=remote,entries=3 if I want 3 reflog entries.
>>
>> I agree that making option-order important is potentially confusing. But
>> it does already exist with --exclude. It's necessary to specify some
>> sets of refs (e.g., all of A, except for those that match B, and then
>> all of C, including those that match B).
>>
>> Having --decorate-reflog=remote would be similarly constrained. You
>> couldn't do "decorate all remotes except for these ones". For that
>> matter, I'm not sure how you would do "decorate just the refs from
>> origin".
>>
>> I'll grant that those are going to be a lot less common than just "all
>> the remotes" (or all the tags, or whatever). I'd just hate to see us
>> revisiting this in a year to generalize it, and being stuck with
>> historical baggage.
>>
>>> My hesitant to go that far is because I suspect decorating reflog
>>> won't be helpful for non-remotes. But I'm willing to make more changes
>>> if it opens door to master.
>>
>> Forgetting reflogs for a moment, I'd actually find it useful to just
>> decorate tags and local branches, but not remotes. But right now there
>> isn't any way to select which refs are worthy of decoration (reflog or
>> not).
>>
>> That's why I'm thinking so much about a general ref-selection system. I
>> agree the "--exclude=... --remotes" thing is complicated, but it's also
>> the ref-selection system we _already_ have, which to me is a slight
>> point in its favor.
>>
>> -Peff
>
> I agree that the interaction between --exclude and --remotes/etc is
> confusing, but I think it's reasonable enough because we already
> support it, so it makes sense to extend it with this. I also think its
> better to extend here than it is to hard-code it.
OK. Next question, how do we deal with the reflog count (i..e the
argument of --decorate-remote-reflog). Should it be shared for all ref
type, or can be specified differently for remote, local and tags? I'm
leaning towards the former. But I'll wait a bit for ideas before
rewriting the patch.
--
Duy
^ permalink raw reply
* Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard
From: Duy Nguyen @ 2017-01-21 12:44 UTC (permalink / raw)
To: Jeff King, Jacob Keller; +Cc: Git mailing list
In-Reply-To: <20170120192539.7jts6xqzx46unn7y@sigill.intra.peff.net>
On Sat, Jan 21, 2017 at 2:25 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 20, 2017 at 11:16:15AM -0800, Jacob Keller wrote:
>
>> > I recently taught urxvt to recognize sha1s and grab them via keyboard
>> > hints, and I'm finding it quite useful. Here's what it looks like if
>> > you're interested:
>> >
>> > http://peff.net/git-hints.gif
>> >
>> > The hints technique is taken from pentadactyl (which I also use), but
>> > the urxvt port is mine. I'm happy to share the code.
You just gave me a reason to rebuild urxvt with perl support. It
solves my problem with SHA-1 nicely (and fixes another problem with
very large counter in my approach, when you scroll git-log further
down, because I can't know what's on screen or already scrolled past).
Do you think it should be in contrib for better discoverability (than
burying it in the mail archive)? A separate git repo is also ok, but I
guess that's just more burden.
>> I would be interested in the code for this.. I'm curious if I can
>> adapt it to my use of tmux.
Yeah. Let me know if it works out. Would be nice to have this run on
basically any terminal, as long as I run tmux on top.
--
Duy
^ permalink raw reply
* [PATCH] git-p4: Fix git-p4.mapUser on Windows
From: George Vanburgh @ 2017-01-21 12:02 UTC (permalink / raw)
To: git
From: George Vanburgh <gvanburgh@bloomberg.net>
When running git-p4 on Windows, with multiple git-p4.mapUser entries in
git config - no user mappings are applied to the generated repository.
Reproduction Steps:
1. Add multiple git-p4.mapUser entries to git config on a Windows
machine
2. Attempt to clone a p4 repository
None of the user mappings will be applied.
This issue is caused by the fact that gitConfigList,
uses split(os.linesep) to convert the output of
git config --get-all into a list.
On Windows, os.linesep is equal to '\r\n' - however git.exe
returns configuration with a line seperator of '\n'. This leads to
the list returned by gitConfigList containing only one element - which
contains the full output of git config --get-all in string form. This
causes problems for the code introduced to getUserMapFromPerforceServer
in 10d08a1.
This issue should be caught by the test introduced in 10d08a1, and
would require running on Windows to reproduce. When running inside
MinGW/Cygwin, however, os.linesep correctly returns '\n', and everything
works as expected.
The simplest fix for this issue would be to convert the line split logic
inside gitConfigList to use splitlines(), which splits on any standard
line delimiter. However, this function was only introduced in Python
2.7, and would mean a bump in the minimum required version of Python
required to run git-p4. The alternative fix, implemented here, is to use
'\n' as a delimiter, which git.exe appears to output consistently on
Windows anyway.
Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
---
git-p4.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index f427bf6..c134a58 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -656,7 +656,7 @@ def gitConfigInt(key):
def gitConfigList(key):
if not _gitConfig.has_key(key):
s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
- _gitConfig[key] = s.strip().split(os.linesep)
+ _gitConfig[key] = s.strip().split("\n")
if _gitConfig[key] == ['']:
_gitConfig[key] = []
return _gitConfig[key]
--
https://github.com/git/git/pull/319
^ permalink raw reply related
* [PATCH] rebase: pass --signoff option to git am
From: Giuseppe Bilotta @ 2017-01-21 10:49 UTC (permalink / raw)
To: git; +Cc: Giuseppe Bilotta
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
Documentation/git-rebase.txt | 5 +++++
git-rebase.sh | 3 ++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 67d48e6883..e6f0b93337 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -385,6 +385,11 @@ have the long commit hash prepended to the format.
Recreate merge commits instead of flattening the history by replaying
commits a merge commit introduces. Merge conflict resolutions or manual
amendments to merge commits are not preserved.
+
+--signoff::
+ This flag is passed to 'git am' to sign off all the rebased
+ commits (see linkgit:git-am[1]).
+
+
This uses the `--interactive` machinery internally, but combining it
with the `--interactive` option explicitly is generally not a good
diff --git a/git-rebase.sh b/git-rebase.sh
index 48d7c5ded4..e468a061f9 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -34,6 +34,7 @@
autosquash move commits that begin with squash!/fixup! under -i
committer-date-is-author-date! passed to 'git am'
ignore-date! passed to 'git am'
+signoff! passed to 'git am'
whitespace=! passed to 'git apply'
ignore-whitespace! passed to 'git apply'
C=! passed to 'git apply'
@@ -321,7 +322,7 @@ run_pre_rebase_hook ()
--ignore-whitespace)
git_am_opt="$git_am_opt $1"
;;
- --committer-date-is-author-date|--ignore-date)
+ --committer-date-is-author-date|--ignore-date|--signoff)
git_am_opt="$git_am_opt $1"
force_rebase=t
;;
--
2.11.0.585.g56041942c3.dirty
^ permalink raw reply related
* Does it make sense to show tips on blame?
From: Edmundo Carmona Antoranz @ 2017-01-21 5:23 UTC (permalink / raw)
To: Git List
Hi!
I'm having fun with difflame. I added support for an option called
'tips' which would display 1-line summary of a block of added lines
that belong to the same revision, in order to provide context while
reading difflame output without having to run an additional git show
command.
Output is like this (from README.txt, taken from difflame on git
itself, sorry if it's too wide):
diff --git a/fast-import.c b/fast-import.c
index f561ba833..64fe602f0 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2218,13 +2218,17 @@ static uintmax_t do_change_note_fanout(
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2218)
char *fullpath, unsigned int fullpath_len,
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2219)
unsigned char fanout)
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2220) {
-02d0457eb4 (Junio C Hamano 2017-01-10 15:24:26 -0800 2221) struct
tree_content *t = root->tree;
405d7f4af fast-import: properly fanout notes when tree is imported
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2221) struct
tree_content *t;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2222) struct
tree_entry *e, leaf;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2223)
unsigned int i, tmp_hex_sha1_len, tmp_fullpath_len;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2224)
uintmax_t num_notes = 0;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2225)
unsigned char sha1[20];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2226) char
realpath[60];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2227)
405d7f4af fast-import: properly fanout notes when tree is imported
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2228) if (!root->tree)
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2229)
load_tree(root);
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2230) t = root->tree;
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2231)
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2232) for (i
= 0; t && i < t->entry_count; i++) {
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2233)
e = t->entries[i];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2234)
tmp_hex_sha1_len = hex_sha1_len + e->name->str_len;
The tip that was used for revision 405d7f4af on both "blocks" of added
lines can be seen.
The question is if you think it makes sense to add this option to
git-blame itself.
Something like:
git blame --tips README.txt
Thanks in advance
^ permalink raw reply related
* [PATCH v2] show-ref: Allow -d, --head to work with --verify
From: Vladimir Panteleev @ 2017-01-21 1:08 UTC (permalink / raw)
To: git
This is the second take for "[PATCH] show-ref: Allow --head to work
with --verify". Thanks to Junio for his extensive feedback.
^ permalink raw reply
* [PATCH v2 2/4] show-ref: Allow -d to work with --verify
From: Vladimir Panteleev @ 2017-01-21 1:08 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170121010821.25046-1-git@thecybershadow.net>
Use the same output machinery when --verify is absent or present,
which allows tag dereferencing (-d) to work with --verify. This is
useful when the user wishes to avoid the costly iteration of refs.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 3 +--
t/t1403-show-ref.sh | 9 +++++++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 945a483e3..bcdc1a95e 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -205,8 +205,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
if ((starts_with(*pattern, "refs/") ||
(show_head && !strcmp(*pattern, "HEAD"))) &&
!read_ref(*pattern, oid.hash)) {
- if (!quiet)
- show_one(*pattern, &oid);
+ show_ref(*pattern, &oid, 0, NULL);
}
else if (!quiet)
die("'%s' - not a valid ref", *pattern);
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 2fb5dc879..5c540e67f 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -97,6 +97,9 @@ test_expect_success 'show-ref -d' '
git show-ref -d refs/tags/A refs/tags/C >actual &&
test_cmp expect actual &&
+ git show-ref --verify -d refs/tags/A refs/tags/C >actual &&
+ test_cmp expect actual &&
+
echo $(git rev-parse refs/heads/master) refs/heads/master >expect &&
git show-ref -d master >actual &&
test_cmp expect actual &&
@@ -116,6 +119,12 @@ test_expect_success 'show-ref -d' '
test_cmp expect actual &&
test_must_fail git show-ref -d --verify heads/master >actual &&
+ test_cmp expect actual &&
+
+ test_must_fail git show-ref --verify -d A C >actual &&
+ test_cmp expect actual &&
+
+ test_must_fail git show-ref --verify -d tags/A tags/C >actual &&
test_cmp expect actual
'
--
2.11.0
^ permalink raw reply related
* [PATCH v2 1/4] show-ref: Allow --head to work with --verify
From: Vladimir Panteleev @ 2017-01-21 1:08 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170121010821.25046-1-git@thecybershadow.net>
Previously, when --verify was specified, show-ref would use a separate
code path which ignored --head. Thus, "git show-ref HEAD" used with
"--verify" (because the user is not interested in seeing
refs/remotes/origin/HEAD), and used with "--head" (because the user
does not want HEAD to be filtered out), i.e. "git show-ref --head
--verify HEAD", did not work as expected.
Instead of insisting that the input begins with "refs/", allow "HEAD"
when "--head" is given in the codepath that handles "--verify", so
that all valid full refnames including HEAD are passed to the same
output machinery.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 3 ++-
t/t1403-show-ref.sh | 14 ++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 6d4e66900..945a483e3 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -202,7 +202,8 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
while (*pattern) {
struct object_id oid;
- if (starts_with(*pattern, "refs/") &&
+ if ((starts_with(*pattern, "refs/") ||
+ (show_head && !strcmp(*pattern, "HEAD"))) &&
!read_ref(*pattern, oid.hash)) {
if (!quiet)
show_one(*pattern, &oid);
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 7e10bcfe3..2fb5dc879 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -164,4 +164,18 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' '
test_cmp expect actual
'
+test_expect_success 'show-ref --verify --head' '
+ echo $(git rev-parse HEAD) HEAD >expect &&
+ git show-ref --verify --head HEAD >actual &&
+ test_cmp expect actual &&
+
+ >expect &&
+
+ git show-ref --verify --head -q HEAD >actual &&
+ test_cmp expect actual &&
+
+ test_must_fail git show-ref --verify --head -q A >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0
^ permalink raw reply related
* [PATCH v2 3/4] show-ref: Optimize show_ref a bit
From: Vladimir Panteleev @ 2017-01-21 1:08 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170121010821.25046-1-git@thecybershadow.net>
The inner `if (verify)' check was not being used before the preceding
commit, as show_ref was never being called from a code path where
verify was non-zero.
Adding a `!verify' check to the outer if skips an unnecessary string
comparison when verify is non-zero, and show_ref is already called
with a reference exactly matching pattern.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index bcdc1a95e..3cf344d47 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -43,7 +43,7 @@ static int show_ref(const char *refname, const struct object_id *oid,
if (!match)
return 0;
}
- if (pattern) {
+ if (pattern && !verify) {
int reflen = strlen(refname);
const char **p = pattern, *m;
while ((m = *p++) != NULL) {
@@ -54,9 +54,6 @@ static int show_ref(const char *refname, const struct object_id *oid,
continue;
if (len == reflen)
goto match;
- /* "--verify" requires an exact match */
- if (verify)
- continue;
if (refname[reflen - len - 1] == '/')
goto match;
}
--
2.11.0
^ permalink raw reply related
* [PATCH v2 4/4] show-ref: Inline show_one
From: Vladimir Panteleev @ 2017-01-21 1:08 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170121010821.25046-1-git@thecybershadow.net>
As the small function is now only called from a single place, there is
no point in keeping it as a separate function any longer.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 3cf344d47..ec44164d8 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -17,15 +17,6 @@ static int deref_tags, show_head, tags_only, heads_only, found_match, verify,
static const char **pattern;
static const char *exclude_existing_arg;
-static void show_one(const char *refname, const struct object_id *oid)
-{
- const char *hex = find_unique_abbrev(oid->hash, abbrev);
- if (hash_only)
- printf("%s\n", hex);
- else
- printf("%s %s\n", hex, refname);
-}
-
static int show_ref(const char *refname, const struct object_id *oid,
int flag, void *cbdata)
{
@@ -74,7 +65,11 @@ static int show_ref(const char *refname, const struct object_id *oid,
if (quiet)
return 0;
- show_one(refname, oid);
+ hex = find_unique_abbrev(oid->hash, abbrev);
+ if (hash_only)
+ printf("%s\n", hex);
+ else
+ printf("%s %s\n", hex, refname);
if (!deref_tags)
return 0;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] show-ref: Allow --head to work with --verify
From: Vladimir Panteleev @ 2017-01-21 0:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqo9z1ux7r.fsf@gitster.mtv.corp.google.com>
On 2017-01-20 23:29, Junio C Hamano wrote:
> By the way, I have no strong preference between "read-ref, check
> quiet and show-one" and "show-ref", so if you make --verify to
> consistently call "show_ref()" for both refs/heads/master and HEAD,
> that is also perfectly fine.
This sounds like a good idea, as it will also allow -d and some other
options to work with --verify (whatever use case there might be for
that). Will resubmit with that.
> I just do not want to see the same feature/codepath to call two
> different implementations that happens to do the same thing today.
Understood and agreed.
--
Best regards,
Vladimir
^ permalink raw reply
* Re: Idea: Add a filter option to 'git rebase'
From: Thomas Braun @ 2017-01-20 23:35 UTC (permalink / raw)
To: Philip Oakley, Git List; +Cc: Johannes Schindelin
In-Reply-To: <8AED6D90D2B64AE3A63C6195CA983FE8@PhilipOakley>
Am 20.01.2017 um 23:28 schrieb Philip Oakley:
> A recent question on stackoverflow
> http://stackoverflow.com/questions/41753252/drop-commits-by-commit-message-in-git-rebase
> sought to remove automatically commits that could be identified by
> relevant words in the commit message.
>
> I had thought that the ubiquitous `git filter-branch` should be able to
> do this sort of thing. I was wrong. (It was pointed out to me that...)
> The man page notes that removing a commit via filter-branch does not
> remove the changes from following commits and directs readers to using
> `git rebase(1)`.
>
> However the rebase command does not have any filter option to allow the
> automatic population of its TODO list with the appropriate
> pick/edit/drop/etc. values.
Well you can use an arbitrary shell command as editor, so something like
$ GIT_SEQUENCE_EDITOR="sed -i -re 's/^pick /edit /'" git rebase -i master
will change pick to edit of all commits.
Maybe that can be mentioned in the man page of rebase?
^ permalink raw reply
* Re: [PATCH v2 2/2] grep: use '/' delimiter for paths
From: Brandon Williams @ 2017-01-20 23:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Hajnoczi, git, Jeff King
In-Reply-To: <xmqqpojhwf2r.fsf@gitster.mtv.corp.google.com>
On 01/20, Junio C Hamano wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
>
> > If the tree contains a sub-directory then git-grep(1) output contains a
> > colon character instead of a path separator:
> >
> > $ git grep malloc v2.9.3:t
> > v2.9.3:t:test-lib.sh: setup_malloc_check () {
> > $ git show v2.9.3:t:test-lib.sh
> > fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3'
> >
> > This patch attempts to use the correct delimiter:
> >
> > $ git grep malloc v2.9.3:t
> > v2.9.3:t/test-lib.sh: setup_malloc_check () {
> > $ git show v2.9.3:t/test-lib.sh
> > (success)
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > builtin/grep.c | 4 +++-
> > t/t7810-grep.sh | 5 +++++
> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/builtin/grep.c b/builtin/grep.c
> > index 90a4f3d..7a7aab9 100644
> > --- a/builtin/grep.c
> > +++ b/builtin/grep.c
> > @@ -494,7 +494,9 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
> >
> > /* Add a delimiter if there isn't one already */
> > if (name[len - 1] != '/' && name[len - 1] != ':') {
> > - strbuf_addch(&base, ':');
> > + /* rev: or rev:path/ */
> > + char delim = obj->type == OBJ_COMMIT ? ':' : '/';
>
> Why check the equality with commit, rather than un-equality with
> tree? Wouldn't you want to treat $commit:path and $tag:path the
> same way?
I assume Stefan just grabbed my naive suggestion hence why it checks
equality with a commit. So that's my fault :) Either of these may
not be enough though, since if you do 'git grep malloc v2.9.3^{tree}'
with this change the output prefix is 'v2.9.3^{tree}/' instead of the
correct prefix 'v2.9.3^{tree}:'
>
> I also think the two patches should be squashed together, as it is
> only after this patch that the code does the "right thing" by
> changing the delimiter depending on obj->type.
>
> > + strbuf_addch(&base, delim);
> > }
> > }
> > init_tree_desc(&tree, data, size);
> > diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> > index e804a3f..8a58d5e 100755
> > --- a/t/t7810-grep.sh
> > +++ b/t/t7810-grep.sh
> > @@ -1445,6 +1445,11 @@ test_expect_success 'grep outputs valid <rev>:<path> for HEAD:t/' '
> > test_cmp expected actual
> > '
> >
> > +test_expect_success 'grep outputs valid <rev>:<path> for HEAD:t' '
> > + git grep vvv HEAD:t >actual &&
> > + test_cmp expected actual
> > +'
> > +
>
> Perhaps you want an annotated tag object refs/tags/v1.0 that points
> at the commit at the HEAD, and then run the same grep on v1.0:t, in
> addition to the above test.
>
> > cat >expected <<EOF
> > HEAD:t/a/v:vvv
> > HEAD:t/v:vvv
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH] show-ref: Allow --head to work with --verify
From: Junio C Hamano @ 2017-01-20 23:29 UTC (permalink / raw)
To: Vladimir Panteleev; +Cc: Vladimir Panteleev, git
In-Reply-To: <xmqqshoduxnj.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> If two codepaths are called "I don't see a meaningful difference",
> then it is really better to share the same code. Today, they may
> happen to behave identically. When we need to update the behaviour
> of one, we'd be forced to update the other one to match.
>
> IOW, something along this line, perhaps (not even compile tested so
> take it with grain of salt).
By the way, I have no strong preference between "read-ref, check
quiet and show-one" and "show-ref", so if you make --verify to
consistently call "show_ref()" for both refs/heads/master and HEAD,
that is also perfectly fine.
I just do not want to see the same feature/codepath to call two
different implementations that happens to do the same thing today.
Thanks.
^ permalink raw reply
* Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.
From: Stefan Beller @ 2017-01-20 23:22 UTC (permalink / raw)
To: Junio C Hamano, Brandon Williams, Jonathan Nieder
Cc: Jeff King, git@vger.kernel.org
In-Reply-To: <xmqqy3y5wg0l.fsf@gitster.mtv.corp.google.com>
On Fri, Jan 20, 2017 at 1:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Jeff King <peff@peff.net> writes:
>>
>>>> And in my current understanding of submodules the check in
>>>> .gitmodules ought to be enough, too.
>>>
>>> Yeah, that probably makes sense. You can have a gitlink without a
>>> .gitmodules file, but I don't quite know what that would mean in terms
>>> of submodules (I guess it's not a submodule but "something else").
>>
>> That may be a lot better than reading the index unconditionally, but
>> I'd rather not to see "git rev-parse" read ".gitmodules" at all. It
>> would discourage scripted use of Git for no good reason.
>
> Thinking about this more, I suspect that
>
> cd sub && git anything
>
> when the index of the top-level thinks "sub" must be a submodule and
> the user is not interested in "sub" (hence it hasn't gone through
> "git submodule init" or "update") should get the same error as you
> would get if you did
>
> cd /var/tmp/ && git anything
>
> when none of /, /var, /var/tmp/ is controlled by any Git repository.
> I.e. "fatal: Not a git repository".
I agree. The idea with a tombstone sounds great from a
performance perspective as you do not need to do extra work
in the superproject at all, because any gitlink is detected early
in the discovery.
The big BUT is however the following:
How do current users know if a submodule is e.g. populated?
(From say a third party script). Most likely they use something like
test -e ${sub}/.git
as that just worked. So if we go with the tombstone idea, we
may break things. (i.e. the fictional third party script confirms any
submodule to be there, but it is not)
I do really like the idea though, so maybe we also need to provide
some submodule plumbing that we opine to be the "correct" way
to see the submodules state[1] to make the transition easier for the
script writers?
[1] c.f. submodule states in
https://github.com/gitster/git/commit/e2b51b9df618ceeff7c4ec044e20f5ce9a87241e
>
> Perhaps we can update two things and make it cheap.
>
> - checking out the top-level working tree without populating the
> working tree of a submodule learns to do a bit more than just
> creating an empty directory. Instead, it creates the third kind
> of ".git" (we currently support two kinds of ".git", one that is
> a repository itself, and another that is points at a repository),
> that tells us that there is not (yet) a repository there.
>
> - the "discovering the root of the working tree" logic learns to
> notice the third kind of ".git" and stop with "Not a git
> repository".
^ permalink raw reply
* Re: [PATCH] show-ref: Allow --head to work with --verify
From: Junio C Hamano @ 2017-01-20 23:20 UTC (permalink / raw)
To: Vladimir Panteleev; +Cc: Vladimir Panteleev, git
In-Reply-To: <3b1d2717-dd7f-2add-b935-3ace6063b258@gmail.com>
Vladimir Panteleev <thecybershadow@gmail.com> writes:
> --quiet will still work correctly with the current patch, because
> show_ref already checks quiet. Granted, the original --verify code
> used show_one and not show_ref; however, I don't see a meaningful
> difference between calling show_ref and show_one for HEAD, other than
> a bit of overhead, so adding a new function may not be worthwhile. I
> will still add tests for this; however, in light of this, would you
> still like me to perform the change you requested?
If two codepaths are called "I don't see a meaningful difference",
then it is really better to share the same code. Today, they may
happen to behave identically. When we need to update the behaviour
of one, we'd be forced to update the other one to match.
IOW, something along this line, perhaps (not even compile tested so
take it with grain of salt).
Thanks.
builtin/show-ref.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 6d4e669002..57491152b7 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -202,7 +202,8 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
while (*pattern) {
struct object_id oid;
- if (starts_with(*pattern, "refs/") &&
+ if (((show_head && !strcmp(*pattern, "HEAD")) ||
+ starts_with(*pattern, "refs/")) &&
!read_ref(*pattern, oid.hash)) {
if (!quiet)
show_one(*pattern, &oid);
^ permalink raw reply related
* Re: [PATCH v2 0/2] grep: make output consistent with revision syntax
From: Junio C Hamano @ 2017-01-20 23:11 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: git, Jeff King, Brandon Williams
In-Reply-To: <20170120171126.16269-1-stefanha@redhat.com>
Stefan Hajnoczi <stefanha@redhat.com> writes:
> v2:
> * Use obj->type instead of re-parsing name for delimiter
> (Followed Brandon's suggestion but named the variable 'delim' since that
> name is used in other places and 'del' is used for deletion.)
> * Add tests
> * Update Patch 1 commit description with a more relevant example
> * PATCH instead of RFC, now works with all documented git-rev-parse(1) syntax
>
> git-grep(1)'s output is not consistent with git-rev-parse(1) revision syntax.
While I was queuing this series (which I think should become a
single patch in the final version), I was trying to see how it
should be described in the release note (aka an entry in the
periodicall "What's cooking" report). Here is how I explained it.
You may want to borrow some parts of the description, especially the
part that talks about <tree-ish>:<path> being a way to name a blob,
when updating the commit log message.
"git grep", when fed a tree-ish as an input, shows each hit
prefixed with "<tree-ish>:<path>:<lineno>:". As <tree-ish> is
almost always either a commit or a tag that points at a commit,
the early part of the output "<tree-ish>:<path>" can be used as
the name of the blob and given to "git show".
When <tree-ish> is a tree given in the extended SHA-1 syntax
(e.g. "<commit>:", or "<commit>:<dir>"), however, this results
in a string that does not name a blob (e.g. "<commit>::<path>"
or "<commit>:<dir>:<path>"). "git grep" has been taught to be
a bit more intelligent about these cases and omit a colon (in
the former case) or use slash (in the latter case) to produce
"<commit>:<path>" and "<commit>:<dir>/<path>" that can be used
as the name of a blob.
As a release note entry is written in a style different from the
commit log message, you would need to adjust the voice of the last
sentence (i.e. "Teach 'git grep' to ..." to give an order to the
codebase), but otherwise the above would make an understandable
justification for the change suitable in a log message.
^ permalink raw reply
* Re: [PATCH] show-ref: Allow --head to work with --verify
From: Junio C Hamano @ 2017-01-20 23:15 UTC (permalink / raw)
To: Vladimir Panteleev; +Cc: git
In-Reply-To: <xmqqefzxwew9.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Your log message for the patch needs to be updated by summarizing
> the above better.
Here is my attempt.
"git show-ref HEAD" used with "--verify" (because the user is
not interested in seeing refs/remotes/origin/HEAD), and used
with "--head" (because the user does not want HEAD to be
filtered out), i.e. "git show-ref --head --verify HEAD", did
not work as expected.
Instead of insisting that the input begins with "refs/", allow
"HEAD" when "--head" is given in the codepath that handles
"--verify", so that all valid full refnames including HEAD are
passed to the same output machinery.
^ permalink raw reply
* Re: [RFC 2/2] grep: use '/' delimiter for paths
From: Junio C Hamano @ 2017-01-20 22:56 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Hajnoczi, git
In-Reply-To: <20170120141954.xyocl6oqoykqmpl5@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> It's not ignored; just as with git-log, it's a pathspec to limit the
> diff. E.g.:
>
> $ git show --name-status v2.9.3
> ...
> M Documentation/RelNotes/2.9.3.txt
> M Documentation/git.txt
> M GIT-VERSION-GEN
>
> $ git show --name-status v2.9.3 -- Documentation
> M Documentation/RelNotes/2.9.3.txt
> M Documentation/git.txt
>
> That's typically less useful than it is with log (where limiting the
> diff also kicks in history simplification and omits some commits
> entirely). But it does do something.
I think Stefan is missing the fact that the argument to "git show
<tree-ish>:<path>" actually is naming a blob that sits at the <path>
in the <tree-ish>. In other words, "show" is acting as a glorified
"git -p cat-file blob", in that use.
The use of "git show" you are demonstrating is still about showing
the commit object, whose behaviour is defined to show the log
message and the diff relative to its sole parent, limited to the
paths that match the pathspec.
It is perfectly fine and desirable that "git show <commit>:<path>"
and "git show <commit> -- <path>" behaves differently. These are
two completely different features.
^ permalink raw reply
* Re: [PATCH] show-ref: Allow --head to work with --verify
From: Vladimir Panteleev @ 2017-01-20 22:55 UTC (permalink / raw)
To: Junio C Hamano, Vladimir Panteleev; +Cc: git
In-Reply-To: <xmqqa8aly2o4.fsf@gitster.mtv.corp.google.com>
On 2017-01-20 19:03, Junio C Hamano wrote:
> and viewed in the wider context, I notice that quiet is not honored
> in the added code. I think that is easily fixable by replacing this
> hunk with something like:
--quiet will still work correctly with the current patch, because
show_ref already checks quiet. Granted, the original --verify code used
show_one and not show_ref; however, I don't see a meaningful difference
between calling show_ref and show_one for HEAD, other than a bit of
overhead, so adding a new function may not be worthwhile. I will still
add tests for this; however, in light of this, would you still like me
to perform the change you requested?
--
Best regards,
Vladimir
^ 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