Re: [PATCH] git-send-email: fix get_maintainer.pl regression
On Thu, Nov 16, 2017 at 10:48 AM, Alex Bennéewrote: > Getting rid of Mail::Address regressed behaviour with common > get_maintainer scripts such as the Linux kernel. Fix the missed corner > case and add a test for it. It is not at all clear, based upon this text, what this is fixing. When you re-roll, please provide a description of the regression in sufficient detail for readers to easily understand the problem and the solution. More below... > Signed-off-by: Alex Bennée > --- > diff --git a/t/t9000/test.pl b/t/t9000/test.pl > @@ -35,7 +35,9 @@ my @success_list = (q[Jane], > q['Jane 'Doe' ], > q[Jane@:;\.,()<>Doe ], > q[Jane Doe], > - q[ Jane Doe]); > + q[ Jane Doe], > + q[j...@example.com (open list:for thing (foo/bar))], > +); Style nit: The dangling ");" introduced by this change differs from the other list(s) in this file which have the closing ");" on the same line as the last item in the list. > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh > +test_expect_success $PREREQ 'setup get_mainter script for cc trailer' " > +cat >expected-cc-script.sh <<-EOF && chmod +x expected-cc-script.sh > +#!/bin/sh > +echo 'One Person (supporter:THIS (FOO/bar))' > +echo 'Two Person (maintainer:THIS THING)' > +echo 'Third List (moderated list:THIS THING (FOO/bar))' > +echo ' (moderated list:FOR THING)' > +echo 'f...@example.com (open list:FOR THING (FOO/bar))' > +echo 's...@example.com (open list)' > +EOF > +" Use write_script() to create the script: --- 8< --- write_script expected-cc-script.sh <<-\EOF && echo '...' ... EOF --- 8< --- No need for #!/bin/sh or chmod, both of which are handled by write_script(). In fact, you could fold this into the following test (since there doesn't seem a good reason for it to be separate). > +test_expect_success $PREREQ 'cc trailer with get_maintainer output' ' > + test_commit cc-trailer && > + clean_fake_sendmail && > + git send-email -1 --to=recipi...@example.com \ > + --cc-cmd="$(pwd)/expected-cc-script.sh" \ > + --smtp-server="$(pwd)/fake.sendmail" && > + test_cmp expected-cc commandline1 > +' > OK I'm afraid I don't fully understand the test harness as this breaks a > bunch of other tests. If anyone can offer some pointers on how to fix > I'd be grateful. There are several problems: * test_commit bombs because there is already a tag named "cc-trailer" created by an earlier test. Fix this by choosing a different name for the commit. Even better would be to avoid making the commit in the first place since it doesn't appear to be relevant to the test, and the test works fine without it. * The directory in which the expected-cc-script.sh is created contains a space; this is intentional to catch bugs in tests and Git itself. In this case, your test is exposing what might be considered a bug in git-send-email itself, in which it invokes the --cc-cmd as "/path/with space/expected-cc-script.sh", which is interpreted as trying to invoke program "/path/with" with argument "space/expected-cc-script.sh". One fix (which you could submit as a preparatory patch, making this a 2-patch series) would be this: --- 8< --- diff --git a/git-send-email.perl b/git-send-email.perl --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1724,7 +1724,7 @@ sub recipients_cmd { my ($prefix, $what, $cmd, $file) = @_; my @addresses = (); -open my $fh, "-|", "$cmd \Q$file\E" + open my $fh, "-|", "\Q$cmd\E \Q$file\E" or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd); while (my $address = <$fh>) { $address =~ s/^\s*//g; --- 8< --- However, it's possible that might break existing users who rely on --cc-cmd="myscript --option arg" working. It's not clear which behavior is correct. * Subsequent tests fail because you've added a commit which they are not expecting. If you look at tests earlier in the file, you will see that they deal with this by removing the added commit (via "git reset --hard HEAD^") by the time the test finishes. However, as noted above, this new test doesn't actually need to make this commit in the first place. So, with fixes, your test might look like this: --- 8< --- test_expect_success $PREREQ 'cc trailer with get_maintainer output' ' test_commit cc-trailer-get-maintainer && test_when_finished "git reset --hard HEAD^" && clean_fake_sendmail && git send-email -1 --to=recipi...@example.com \ --cc-cmd="$(pwd)/expected-cc-script.sh" \ --smtp-server="$(pwd)/fake.sendmail" && test_cmp expected-cc commandline1 ' --- 8< --- Or, if you drop the unnecessary test_commit(): --- 8< --- test_expect_success $PREREQ 'cc trailer with get_maintainer output' '
Re: [PATCH 1/2] Simplify tracing code by removing trace key normalization concept
gennady.kup...@gmail.com writes: > Subject: Re: [PATCH 1/2] Simplify tracing code by removing trace key > normalization concept The usual style comment on the subject applies here. > From: Gennady Kupava> > - to implement efficient traces with normalization, normalization > implementation should be moved to header. as it seems better to not > overload header file with this normalization logic, suggestion is > just to remove it > - different macro exist specifically to handle traces with default key > - there is no use of normalization in current code > - it could be reintroduced if necessary I cannot quite tell what it is trying to achive to make it a bulleted list. It's not like four things at the same conceptual level is enumerated; instead it just has four sentences that talk about random things. More importantly, I am not sure I understand what these sentences are trying to say. "Should be moved to header"---so? Does that move something from the source to the header? It seems to me that the patch removes a helper function from trace.c but does not add anything to the header. Or am I wasting everybody's time by commenting on a stale comment that used to describe an ancient iteration of this code? Puzzled.
Re: [PATCH] git-rebase: clean up dashed-usages in messages
> git rebase [-i] [options] [--exec ] [--onto ] [] > [] > git rebase [-i] [options] [--exec ] [--onto ] --root [] > -git-rebase --continue | --abort | --skip | --edit-todo > +git rebase --continue | --abort | --skip | --edit-todo A good change. > test -f "$apply_dir"/applying && > - die "$(gettext "It looks like git-am is in progress. Cannot rebase.")" > + die "$(gettext "It looks like you are in the middle of an am session. > Cannot rebase.")" Probably not, as 'am' alone would be confusing. "It looks like 'git am' is in progress. Cannot rebase." may be a more sensible improvement.
v2.15.0: commits become falsely joined when rebasing (interactively)
Hello again, i see an error with v2.15.0 that happened already back in early October (AlpineLinux [edge] pretty much up-to-date with newest git but please don't ask exact version). I failed to reproduce it back then, but now again, here is how. - It seems related to having a hook (pre-commit), and the "reword" action. - Doing a rebase interactively to move (two) commits back from HEAD downwards in a linear hierarchy, as in * 306b5c7e (HEAD -> refs/heads/notpushed) [-] COMMIT 1 * 7e34d5fa [-] COMMIT 2 the above two to be moved down * 65d216c3 (refs/remotes/origin/notpushed)... ... * 5ff5ef05 (refs/remotes/origin/next, refs/heads/next) [-].. to end up stacked upon and as new [next] - COMMIT 2 is to be picked, but COMMIT 1 shall be "r"eworded. Now what happens is that COMMIT 2 is rebased ok, but instead of simply opening the editor to allow rewording of the commit message of COMMIT 2 the pre-commit hook runs, and it has to complain in this case (lines too long), but i "commit -n" that once i committed first. Anyway git says You can amend the commit now, with git commit --amend Once you are satisfied with your changes, run git rebase --continue Which i have not asked for! More: ?1[steffen@essex nail.git]$ git status interactive rebase in progress; onto 6d437ab6 Last commands done (12 commands done): pick 7e34d5fa [-] COMMIT 2 r 306b5c7e [-] COMMIT 1 (see more in file .git/rebase-merge/done) Next commands to do (27 remaining commands): ... (use "git rebase --edit-todo" to view and edit) You are currently rebasing branch 'notpushed' on '6d437ab6'. (all conflicts fixed: run "git rebase --continue") Changes to be committed: (use "git reset HEAD ..." to unstage) modified: gen-okeys.h Untracked files not listed (use -u option to show untracked files) So far so good, but now: ?0[steffen@essex nail.git]$ git rec alias: rebase --continue [detached HEAD ca77a94a] COMMIT 1 with adjusted commit message Date: Sun Nov 19 02:19:30 2017 +0100 9 files changed, 357 insertions(+), 339 deletions(-) Successfully rebased and updated refs/heads/notpushed. Uh! It joined COMMIT 1 with COMMIT 2! Thanks for git! Ciao, --steffen | |Der Kragenbaer,The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt)
Re: [PATCH 1/6] t4051: add test for comments preceding function lines
On Sat, Nov 18, 2017 at 1:04 PM, René Scharfewrote: > When showing function context it would be helpful to show comments > immediately before declarations, as they are most likely relevant. Add > a test for that. > > Signed-off-by: Rene Scharfe > --- > diff --git a/t/t4051-diff-function-context.sh > b/t/t4051-diff-function-context.sh > @@ -85,6 +85,10 @@ test_expect_success 'setup' ' >> +test_expect_failure ' context includes comment' ' > + grep "^ .*Hello comment" changed_hello.diff > +' This shows only that the the "* Hello comment." line is in the context but says nothing about the entire comment block being included (which patch 3/6 implements). Perhaps the test could be more thorough? > diff --git a/t/t4051/hello.c b/t/t4051/hello.c > index 63b1a1e4ef..73e767e178 100644 > --- a/t/t4051/hello.c > +++ b/t/t4051/hello.c > @@ -1,4 +1,7 @@ > > +/* > + * Hello comment. > + */ > static void hello(void)// Begin of hello > {
Re: [PATCH] launch_editor(): indicate that Git waits for user input
Kaartic Sivaraamwrites: > $ GIT_EDITOR=gedit ./git commit --allow-empty > Launched your editor, waiting...error: gedit died of signal 15 > error: There was a problem with the editor 'gedit'. > Please supply the message using either -m or -F option. > > Though this is not something that's going to happen very often, I'm > not sure if this is to be considered. Just wanted to note what I > observed. See my earlier response to Eric Sunshine. >> +static const char *close_notice = NULL; > > Just a little curious, what's the advantage of making 'close_notice' > 'static' over 'auto' ? A second call, if an earlier call to the function already determined that our standard error output is going to a terminal and what kind of terminal we are on, would just use the result from the earliser call. > Being curious again, is flushing 'stderr' required ? AFAIK, 'stderr' > is unbuffered by default and I didn't notice any calls that changed > the buffering mode of it along this code path. "By default" is the key phrase. The code is merely being defensive to changes in other area of the code.
Re: [PATCH 4/4] builtin/branch: strip refs/heads/ using skip_prefix
On Sat, Nov 18, 2017 at 12:26 PM, Kaartic Sivaraamwrote: > Instead of hard-coding the offset strlen("refs/heads/") to skip > the prefix "refs/heads/" use the skip_prefix() function which > is more communicative and verifies that the string actually > starts with that prefix. > > Though we don't check for the result of verification here as > it's (almost) always the case that the string does start > with "refs/heads", it's just better to avoid hard-coding and > be more communicative. > > Signed-off-by: Kaartic Sivaraam > --- > diff --git a/builtin/branch.c b/builtin/branch.c > @@ -462,6 +462,8 @@ static void copy_or_rename_branch(const char *oldname, > const char *newname, int > { > struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = > STRBUF_INIT; > struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT; > + const char *prefix_free_oldref = NULL; > + const char *prefix_free_newref = NULL; A bit of a mouthful. Perhaps name these 'oldname' and 'newname' or something?
Re: [PATCH] git-send-email: honor $PATH
"brian m. carlson"writes: > This patch adds support for PATH, but it also removes the fixed paths. > On many systems, unprivileged users don't have /usr/sbin in their PATH, > and I know of no systems which provide /usr/lib as a PATH value. > Therefore, it's possible that this change will break automatic detection > of sendmail for many users. It is more than possible ;-) that this change alone is a regression. > I think what you probably want to do is use entries in PATH first, and > leave the two old values as backups at the end. I do not think it would make things worse if the change were to do the two standard places first and then try elements on the $PATH; split of $PATH needs to be done carefully (Windows?), though. I would feel a lot more worried about trying elements on the $PATH first and then using the two standard places as fallback. If the order of addition matters at all, that would mean that trying elements on $PATH first and then falling back to the two standard places *will* change the behaviour---for the affected users, we used to pick one of these two, but now we would pick something different. sendmail is usually installed out of the way of $PATH for regular users for a reason, so picking anything whose name happens to be sendmail that is on $PATH does not sound right. Of course, for users who do not have sendmail at one of the two standard places _and_ has one on one of the directories on $PATH, the order in which we check would not make a difference, so my suggestion would be to do the other way around.
[PATCH 2/2] Reduce performance cost of the trace if trace category is disabled
From: Gennady Kupava- Do the check if the trace key is enabled sooner in call chain. - Move just enough code from trace.c into trace.h header so all code necessary to determine that trace is disabled could be inlined to calling functions. Signed-off-by: Gennady Kupava --- trace.c | 3 +-- trace.h | 58 -- 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/trace.c b/trace.c index d47ea28e8..b7530b51a 100644 --- a/trace.c +++ b/trace.c @@ -25,6 +25,7 @@ #include "quote.h" struct trace_key trace_default_key = { "GIT_TRACE", 0, 0, 0 }; +struct trace_key trace_perf_key = TRACE_KEY_INIT(PERFORMANCE); /* Get a trace file descriptor from "key" env variable. */ static int get_trace_fd(struct trace_key *key) @@ -172,8 +173,6 @@ void trace_strbuf_fl(const char *file, int line, struct trace_key *key, print_trace_line(key, ); } -static struct trace_key trace_perf_key = TRACE_KEY_INIT(PERFORMANCE); - static void trace_performance_vprintf_fl(const char *file, int line, uint64_t nanos, const char *format, va_list ap) diff --git a/trace.h b/trace.h index 24b32f8f4..cd9e280ba 100644 --- a/trace.h +++ b/trace.h @@ -14,6 +14,7 @@ struct trace_key { extern struct trace_key trace_default_key; #define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name, 0, 0, 0 } +extern struct trace_key trace_perf_key; extern void trace_repo_setup(const char *prefix); extern int trace_want(struct trace_key *key); @@ -79,24 +80,42 @@ extern void trace_performance_since(uint64_t start, const char *format, ...); * comma, but this is non-standard. */ -#define trace_printf(...) \ - trace_printf_key_fl(TRACE_CONTEXT, __LINE__, _default_key, __VA_ARGS__) - -#define trace_printf_key(key, ...) \ - trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key, __VA_ARGS__) - -#define trace_argv_printf(argv, ...) \ - trace_argv_printf_fl(TRACE_CONTEXT, __LINE__, argv, __VA_ARGS__) - -#define trace_strbuf(key, data) \ - trace_strbuf_fl(TRACE_CONTEXT, __LINE__, key, data) - -#define trace_performance(nanos, ...) \ - trace_performance_fl(TRACE_CONTEXT, __LINE__, nanos, __VA_ARGS__) - -#define trace_performance_since(start, ...) \ - trace_performance_fl(TRACE_CONTEXT, __LINE__, getnanotime() - (start), \ -__VA_ARGS__) +#define trace_printf_key(key, ...) \ + do {\ + if (trace_pass_fl(key)) \ + trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key, \ + __VA_ARGS__); \ + } while(0) + +#define trace_printf(...) trace_printf_key(_default_key, __VA_ARGS__); + +#define trace_argv_printf(argv, ...) \ + do {\ + if (trace_pass_fl(_default_key)) \ + trace_argv_printf_fl(TRACE_CONTEXT, __LINE__,\ + argv, __VA_ARGS__); \ + } while(0) + +#define trace_strbuf(key, data) \ + do {\ + if (trace_pass_fl(key)) \ + trace_strbuf_fl(TRACE_CONTEXT, __LINE__, key, data);\ + } while(0) + +#define trace_performance(nanos, ...) \ + do {\ + if (trace_pass_fl(key)) \ + trace_performance_fl(TRACE_CONTEXT, __LINE__, nanos,\ +__VA_ARGS__); \ + } while(0) + +#define trace_performance_since(start, ...)\ + do {\ + if (trace_pass_fl(_perf_key)) \ + trace_performance_fl(TRACE_CONTEXT, __LINE__, \ +getnanotime() - (start), \ +__VA_ARGS__); \ + } while(0) /* backend functions, use non-*fl macros instead */ __attribute__((format (printf, 4, 5))) @@ -110,6 +129,9 @@ extern void trace_strbuf_fl(const char *file, int line, struct trace_key *key, __attribute__((format (printf, 4, 5))) extern void trace_performance_fl(const char *file, int line, uint64_t nanos, const char *fmt, ...); +inline int
[PATCH 1/2] Simplify tracing code by removing trace key normalization concept
From: Gennady Kupava- to implement efficient traces with normalization, normalization implementation should be moved to header. as it seems better to not overload header file with this normalization logic, suggestion is just to remove it - different macro exist specifically to handle traces with default key - there is no use of normalization in current code - it could be reintroduced if necessary Signed-off-by: Gennady Kupava --- trace.c | 24 trace.h | 4 +++- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/trace.c b/trace.c index cb1293ed3..d47ea28e8 100644 --- a/trace.c +++ b/trace.c @@ -24,26 +24,13 @@ #include "cache.h" #include "quote.h" -/* - * "Normalize" a key argument by converting NULL to our trace_default, - * and otherwise passing through the value. All caller-facing functions - * should normalize their inputs in this way, though most get it - * for free by calling get_trace_fd() (directly or indirectly). - */ -static void normalize_trace_key(struct trace_key **key) -{ - static struct trace_key trace_default = { "GIT_TRACE" }; - if (!*key) - *key = _default; -} +struct trace_key trace_default_key = { "GIT_TRACE", 0, 0, 0 }; /* Get a trace file descriptor from "key" env variable. */ static int get_trace_fd(struct trace_key *key) { const char *trace; - normalize_trace_key(); - /* don't open twice */ if (key->initialized) return key->fd; @@ -81,8 +68,6 @@ static int get_trace_fd(struct trace_key *key) void trace_disable(struct trace_key *key) { - normalize_trace_key(); - if (key->need_close) close(key->fd); key->fd = 0; @@ -128,7 +113,6 @@ static int prepare_trace_line(const char *file, int line, static void trace_write(struct trace_key *key, const void *buf, unsigned len) { if (write_in_full(get_trace_fd(key), buf, len) < 0) { - normalize_trace_key(); warning("unable to write trace for %s: %s", key->key, strerror(errno)); trace_disable(key); @@ -167,13 +151,13 @@ static void trace_argv_vprintf_fl(const char *file, int line, { struct strbuf buf = STRBUF_INIT; - if (!prepare_trace_line(file, line, NULL, )) + if (!prepare_trace_line(file, line, _default_key, )) return; strbuf_vaddf(, format, ap); sq_quote_argv(, argv, 0); - print_trace_line(NULL, ); + print_trace_line(_default_key, ); } void trace_strbuf_fl(const char *file, int line, struct trace_key *key, @@ -215,7 +199,7 @@ void trace_printf(const char *format, ...) { va_list ap; va_start(ap, format); - trace_vprintf_fl(NULL, 0, NULL, format, ap); + trace_vprintf_fl(NULL, 0, _default_key, format, ap); va_end(ap); } diff --git a/trace.h b/trace.h index 179b249c5..24b32f8f4 100644 --- a/trace.h +++ b/trace.h @@ -11,6 +11,8 @@ struct trace_key { unsigned int need_close : 1; }; +extern struct trace_key trace_default_key; + #define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name, 0, 0, 0 } extern void trace_repo_setup(const char *prefix); @@ -78,7 +80,7 @@ extern void trace_performance_since(uint64_t start, const char *format, ...); */ #define trace_printf(...) \ - trace_printf_key_fl(TRACE_CONTEXT, __LINE__, NULL, __VA_ARGS__) + trace_printf_key_fl(TRACE_CONTEXT, __LINE__, _default_key, __VA_ARGS__) #define trace_printf_key(key, ...) \ trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key, __VA_ARGS__) -- 2.14.1
[PATCH v3 2/3] worktree: make add dwim
Currently 'git worktree add ', errors out when 'branch' is not a local branch. It has no additional dwim'ing features that one might expect. Make it behave more like 'git checkout ' when the branch doesn't exist locally, but a remote tracking branch uniquely matches the desired branch name, i.e. create a new branch from the remote tracking branch and set the upstream to the remote tracking branch. As 'git worktree add' currently just dies in this situation, there are no backwards compatibility worries when introducing this feature. Signed-off-by: Thomas Gummerer--- Documentation/git-worktree.txt | 7 +++ builtin/worktree.c | 15 ++ t/t2025-worktree-add.sh| 46 ++ 3 files changed, 68 insertions(+) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index b472acc356..abf48fecb8 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -52,6 +52,13 @@ is linked to the current repository, sharing everything except working directory specific files such as HEAD, index, etc. `-` may also be specified as ``; it is synonymous with `@{-1}`. + +If is not found, and neither `-b` nor `-B` nor `--detach` are +used, but there does exist a tracking branch in exactly one remote +(call it ) with a matching name, treat as equivalent to + +$ git worktree add -b / + ++ If `` is omitted and neither `-b` nor `-B` nor `--detach` used, then, as a convenience, a new branch based at HEAD is created automatically, as if `-b $(basename )` was specified. diff --git a/builtin/worktree.c b/builtin/worktree.c index 7b9307aa58..05fc902bcc 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "checkout.h" #include "config.h" #include "builtin.h" #include "dir.h" @@ -386,6 +387,20 @@ static int add(int ac, const char **av, const char *prefix) opts.new_branch = xstrndup(s, n); } + if (ac == 2 && !opts.new_branch && !opts.detach) { + struct object_id oid; + struct commit *commit; + const char *remote; + + commit = lookup_commit_reference_by_name(branch); + if (!commit) + remote = unique_tracking_name(branch, ); + if (!commit && remote) { + opts.new_branch = branch; + branch = remote; + } + } + if (opts.new_branch) { struct child_process cp = CHILD_PROCESS_INIT; cp.git_cmd = 1; diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index b5c47ac602..e3fc60dd1c 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -6,6 +6,16 @@ test_description='test git worktree add' . "$TEST_DIRECTORY"/lib-rebase.sh +# Is branch "refs/heads/$1" set to pull from "$2/$3"? +test_branch_upstream () { + printf "%s\n" "$2" "refs/heads/$3" >expect.upstream && + { + git config "branch.$1.remote" && + git config "branch.$1.merge" + } >actual.upstream && + test_cmp expect.upstream actual.upstream +} + test_expect_success 'setup' ' test_commit init ' @@ -314,4 +324,40 @@ test_expect_success 'rename a branch under bisect not allowed' ' test_must_fail git branch -M under-bisect bisect-with-new-name ' +test_expect_success '"add" fails' ' + test_must_fail git worktree add foo non-existent +' + +test_expect_success '"add" dwims' ' + test_when_finished rm -rf repo_upstream && + test_when_finished rm -rf repo_dwim && + test_when_finished rm -rf foo && + git init repo_upstream && + ( + cd repo_upstream && + test_commit upstream_master && + git checkout -b foo && + test_commit a_foo + ) && + git init repo_dwim && + ( + cd repo_dwim && + test_commit dwim_master && + git remote add repo_upstream ../repo_upstream && + git config remote.repo_upstream.fetch \ + "refs/heads/*:refs/remotes/repo_upstream/*" && + git fetch --all && + test_must_fail git worktree add -b foo ../foo foo && + test_must_fail git worktree add --detach ../foo foo && + git worktree add ../foo foo + ) && + ( + cd foo && + test_branch_upstream foo repo_upstream foo && + git rev-parse repo_upstream/foo >expect && + git rev-parse foo >actual && + test_cmp expect actual + ) +' + test_done -- 2.15.0.345.gf926f18f3
[PATCH v3 1/3] checkout: factor out functions to new lib file
Factor the functions out, so they can be re-used from other places. In particular these functions will be re-used in builtin/worktree.c to make git worktree add dwim more. While there add some docs to the function. Signed-off-by: Thomas Gummerer--- Makefile | 1 + builtin/checkout.c | 41 + checkout.c | 42 ++ checkout.h | 13 + 4 files changed, 57 insertions(+), 40 deletions(-) create mode 100644 checkout.c create mode 100644 checkout.h diff --git a/Makefile b/Makefile index cd75985991..8d603c7443 100644 --- a/Makefile +++ b/Makefile @@ -757,6 +757,7 @@ LIB_OBJS += branch.o LIB_OBJS += bulk-checkin.o LIB_OBJS += bundle.o LIB_OBJS += cache-tree.o +LIB_OBJS += checkout.o LIB_OBJS += color.o LIB_OBJS += column.o LIB_OBJS += combine-diff.o diff --git a/builtin/checkout.c b/builtin/checkout.c index fc4f8fd2ea..9e1cfd10b3 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1,5 +1,6 @@ #include "builtin.h" #include "config.h" +#include "checkout.h" #include "lockfile.h" #include "parse-options.h" #include "refs.h" @@ -874,46 +875,6 @@ static int git_checkout_config(const char *var, const char *value, void *cb) return git_xmerge_config(var, value, NULL); } -struct tracking_name_data { - /* const */ char *src_ref; - char *dst_ref; - struct object_id *dst_oid; - int unique; -}; - -static int check_tracking_name(struct remote *remote, void *cb_data) -{ - struct tracking_name_data *cb = cb_data; - struct refspec query; - memset(, 0, sizeof(struct refspec)); - query.src = cb->src_ref; - if (remote_find_tracking(remote, ) || - get_oid(query.dst, cb->dst_oid)) { - free(query.dst); - return 0; - } - if (cb->dst_ref) { - free(query.dst); - cb->unique = 0; - return 0; - } - cb->dst_ref = query.dst; - return 0; -} - -static const char *unique_tracking_name(const char *name, struct object_id *oid) -{ - struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 }; - cb_data.src_ref = xstrfmt("refs/heads/%s", name); - cb_data.dst_oid = oid; - for_each_remote(check_tracking_name, _data); - free(cb_data.src_ref); - if (cb_data.unique) - return cb_data.dst_ref; - free(cb_data.dst_ref); - return NULL; -} - static int parse_branchname_arg(int argc, const char **argv, int dwim_new_local_branch_ok, struct branch_info *new, diff --git a/checkout.c b/checkout.c new file mode 100644 index 00..b0c744d37a --- /dev/null +++ b/checkout.c @@ -0,0 +1,42 @@ +#include "cache.h" +#include "remote.h" + +struct tracking_name_data { + /* const */ char *src_ref; + char *dst_ref; + struct object_id *dst_oid; + int unique; +}; + +static int check_tracking_name(struct remote *remote, void *cb_data) +{ + struct tracking_name_data *cb = cb_data; + struct refspec query; + memset(, 0, sizeof(struct refspec)); + query.src = cb->src_ref; + if (remote_find_tracking(remote, ) || + get_oid(query.dst, cb->dst_oid)) { + free(query.dst); + return 0; + } + if (cb->dst_ref) { + free(query.dst); + cb->unique = 0; + return 0; + } + cb->dst_ref = query.dst; + return 0; +} + +const char *unique_tracking_name(const char *name, struct object_id *oid) +{ + struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 }; + cb_data.src_ref = xstrfmt("refs/heads/%s", name); + cb_data.dst_oid = oid; + for_each_remote(check_tracking_name, _data); + free(cb_data.src_ref); + if (cb_data.unique) + return cb_data.dst_ref; + free(cb_data.dst_ref); + return NULL; +} diff --git a/checkout.h b/checkout.h new file mode 100644 index 00..9980711179 --- /dev/null +++ b/checkout.h @@ -0,0 +1,13 @@ +#ifndef CHECKOUT_H +#define CHECKOUT_H + +#include "cache.h" + +/* + * Check if the branch name uniquely matches a branch name on a remote + * tracking branch. Return the name of the remote if such a branch + * exists, NULL otherwise. + */ +extern const char *unique_tracking_name(const char *name, struct object_id *oid); + +#endif /* CHECKOUT_H */ -- 2.15.0.345.gf926f18f3
[PATCH v3 0/3] make git worktree add dwim more
Sorry about the noise with v2 and v3 so quickly one after another. I only too late realized that the new dwim for 'add ' doesn't make much sense if -b or --detach are given, and it's better to keep on erroring out in these cases. The previous rounds were at https://public-inbox.org/git/20171112134305.3949-1-t.gumme...@gmail.com/ and https://public-inbox.org/git/20171118181103.28354-1-t.gumme...@gmail.com/. In case anybody already started reading v2, an interdiff between the two versions is below: diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 11cac104d9..eedead2c4c 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -52,9 +52,9 @@ is linked to the current repository, sharing everything except working directory specific files such as HEAD, index, etc. `-` may also be specified as ``; it is synonymous with `@{-1}`. + -If is not found but there does exist a tracking branch in -exactly one remote (call it ) with a matching name, treat as -equivalent to +If is not found, and neither `-b` nor `-B` nor `--detach` are +used, but there does exist a tracking branch in exactly one remote +(call it ) with a matching name, treat as equivalent to $ git worktree add -b / diff --git a/builtin/worktree.c b/builtin/worktree.c index 82088415b8..b2a6dd020c 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -396,7 +396,7 @@ static int add(int ac, const char **av, const char *prefix) } } - if (ac == 2) { + if (ac == 2 && !opts.new_branch && !opts.detach) { struct object_id oid; struct commit *commit; const char *remote; diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index a566d867fe..87e233f812 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -347,6 +347,8 @@ test_expect_success '"add" dwims' ' git config remote.repo_upstream.fetch \ "refs/heads/*:refs/remotes/repo_upstream/*" && git fetch --all && + test_must_fail git worktree add -b foo ../foo foo && + test_must_fail git worktree add --detach ../foo foo && git worktree add ../foo foo ) && ( Thomas Gummerer (3): checkout: factor out functions to new lib file worktree: make add dwim worktree: make add dwim Documentation/git-worktree.txt | 22 +++-- Makefile | 1 + builtin/checkout.c | 41 +--- builtin/worktree.c | 24 ++ checkout.c | 42 checkout.h | 13 + t/t2025-worktree-add.sh| 106 + 7 files changed, 206 insertions(+), 43 deletions(-) create mode 100644 checkout.c create mode 100644 checkout.h -- 2.15.0.345.gf926f18f3
[PATCH v3 3/3] worktree: make add dwim
Currently 'git worktree add ' creates a new branch named after the basename of the , that matches the HEAD of whichever worktree we were on when calling "git worktree add ". Make 'git worktree add behave more like the dwim machinery in 'git checkout ', i.e. check if the new branch name uniquely matches the branch name of a remote tracking branch, and if so check out that branch and set the upstream to the remote tracking branch. This is a change of behaviour compared to the current behaviour, where we create a new branch matching HEAD. However as 'git worktree' is still an experimental feature, and it's easy to notice/correct the behaviour in case it's not what the user desired it's probably okay to break existing behaviour here. In order to also satisfy users who want the current behaviour of creating a new branch from HEAD, add a '--no-track' flag, which disables the new behaviour, and keeps the old behaviour of creating a new branch from the head of the current worktree. Signed-off-by: Thomas Gummerer--- Documentation/git-worktree.txt | 15 --- builtin/worktree.c | 9 +++ t/t2025-worktree-add.sh| 60 ++ 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index abf48fecb8..eedead2c4c 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -60,9 +60,18 @@ $ git worktree add -b / + If `` is omitted and neither `-b` nor `-B` nor `--detach` used, -then, as a convenience, a new branch based at HEAD is created automatically, -as if `-b $(basename )` was specified. - +then, as a convenience, if there exists a tracking branch in exactly +one remote (call it ) matching the basename of the path +(call it ), treat it as equivalent to + +$ git worktree add -b / + +If no tracking branch exists in exactly one remote, is +created based on HEAD, as if `-b $(basename )` was specified. ++ +To disable the behaviour of trying to match the basename of to +a remote, and always create a new branch from HEAD, the `--no-track` +flag can be passed to `git worktree add`. list:: List details of each worktree. The main worktree is listed first, followed by diff --git a/builtin/worktree.c b/builtin/worktree.c index 05fc902bcc..b2a6dd020c 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -342,6 +342,7 @@ static int add(int ac, const char **av, const char *prefix) const char *new_branch_force = NULL; char *path; const char *branch; + int track_dwim = 1; struct option options[] = { OPT__FORCE(, N_("checkout even if already checked out in other worktree")), OPT_STRING('b', NULL, _branch, N_("branch"), @@ -351,6 +352,7 @@ static int add(int ac, const char **av, const char *prefix) OPT_BOOL(0, "detach", , N_("detach HEAD at named commit")), OPT_BOOL(0, "checkout", , N_("populate the new working tree")), OPT_BOOL(0, "lock", _locked, N_("keep the new working tree locked")), + OPT_BOOL(0, "track", _dwim, N_("checkout upstream branch if there's a unique match")), OPT_END() }; @@ -385,6 +387,13 @@ static int add(int ac, const char **av, const char *prefix) int n; const char *s = worktree_basename(path, ); opts.new_branch = xstrndup(s, n); + if (track_dwim) { + struct object_id oid; + const char *remote = + unique_tracking_name(opts.new_branch, ); + if (remote) + branch = remote; + } } if (ac == 2 && !opts.new_branch && !opts.detach) { diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index e3fc60dd1c..87e233f812 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -360,4 +360,64 @@ test_expect_success '"add" dwims' ' ) ' +test_expect_success 'git worktree add --no-track does not set up tracking' ' + test_when_finished rm -rf repo_a && + test_when_finished rm -rf repo_b && + test_when_finished rm -rf foo && + git init repo_a && + ( + cd repo_a && + test_commit a_master && + git checkout -b foo && + test_commit a_foo + ) && + git init repo_b && + ( + cd repo_b && + test_commit b_master && + git remote add repo_a ../repo_a && + git config remote.repo_a.fetch \ + "+refs/heads/*:refs/remotes/other_a/*" && + git fetch --all && + git worktree add --no-track ../foo + ) && + ( + cd foo && + ! test_branch_upstream foo repo_a
git archive --remote should generate tar.gz format indicated by -o filename
git archive -o name.tar.gz generates a gzipped file without needing an explicit --format switch. However, git archive -o name.tar.gz --remote [url] generates a tar file, which is unexpected, bandwidth-heavier, and additionally in some cases it's not immediately obvious that this has happened. git archive -o name.tar.gz --remote [url] --format tar.gz generates a gzipped file, so there's obviously no limitation with e.g. git-upload- archive. Given the above, either git archive or git-upload-archive should apply the same tar.gz filename heuristic and generate the expected format. Presumably e.g. tar.xz support when using --remote would be more problematic since, in the local case, it involves specifying an arbitrary command.
[PATCH v2] git-send-email: honor $PATH for sendmail binary
This extends git-send-email to also consider sendmail binaries in $PATH, in addition to the (fixed) list of /usr/sbin and /usr/lib.fixed) list of paths. Signed-off-by: Florian Klink--- Documentation/git-send-email.txt | 6 +++--- git-send-email.perl | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index bac9014ac..7af48f8eb 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -203,9 +203,9 @@ a password is obtained using 'git-credential'. specify a full pathname of a sendmail-like program instead; the program must support the `-i` option. Default value can be specified by the `sendemail.smtpServer` configuration - option; the built-in default is `/usr/sbin/sendmail` or - `/usr/lib/sendmail` if such program is available, or - `localhost` otherwise. + option; the built-in default is to search in $PATH, + then /usr/sbin and /usr/lib/sendmail afterwards if such program + is available, falling back to `localhost` otherwise. --smtp-server-port=:: Specifies a port different from the default port (SMTP diff --git a/git-send-email.perl b/git-send-email.perl index 2208dcc21..570f04079 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -885,7 +885,9 @@ if (defined $initial_reply_to) { } if (!defined $smtp_server) { - foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) { + my @sendmail_paths = map {"$_/sendmail"} split /:/, $ENV{PATH}; + push @sendmail_paths, qw( /usr/sbin/sendmail /usr/lib/sendmail ); + foreach (@sendmail_paths) { if (-x $_) { $smtp_server = $_; last; -- 2.15.0
Re: [PATCH v2 2/3] worktree: make add dwim
On 11/18, Thomas Gummerer wrote: > Currently 'git worktree add ', errors out when 'branch' > is not a local branch. It has no additional dwim'ing features that one > might expect. > > Make it behave more like 'git checkout ' when the branch doesn't > exist locally, but a remote tracking branch uniquely matches the desired > branch name, i.e. create a new branch from the remote tracking branch > and set the upstream to the remote tracking branch. > > As 'git worktree add' currently just dies in this situation, there are > no backwards compatibility worries when introducing this feature. > > Signed-off-by: Thomas Gummerer> --- > Documentation/git-worktree.txt | 7 +++ > builtin/worktree.c | 15 ++ > t/t2025-worktree-add.sh| 44 > ++ > 3 files changed, 66 insertions(+) > > diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt > index b472acc356..3c7c8c3cee 100644 > --- a/Documentation/git-worktree.txt > +++ b/Documentation/git-worktree.txt > @@ -52,6 +52,13 @@ is linked to the current repository, sharing everything > except working > directory specific files such as HEAD, index, etc. `-` may also be > specified as ``; it is synonymous with `@{-1}`. > + > +If is not found but there does exist a tracking branch in > +exactly one remote (call it ) with a matching name, treat as > +equivalent to > + > +$ git worktree add -b / > + > ++ > If `` is omitted and neither `-b` nor `-B` nor `--detach` used, > then, as a convenience, a new branch based at HEAD is created automatically, > as if `-b $(basename )` was specified. > diff --git a/builtin/worktree.c b/builtin/worktree.c > index 7b9307aa58..92b583ae39 100644 > --- a/builtin/worktree.c > +++ b/builtin/worktree.c > @@ -1,4 +1,5 @@ > #include "cache.h" > +#include "checkout.h" > #include "config.h" > #include "builtin.h" > #include "dir.h" > @@ -386,6 +387,20 @@ static int add(int ac, const char **av, const char > *prefix) > opts.new_branch = xstrndup(s, n); > } > > + if (ac == 2) { Err I just realized this doesn't quite make sense. Similar to the dwim for 'git worktree add ', this condition should include '!opts.new_branch && !opts.detach'. In these cases it's still better to error out, as the user explicitly asked for a new branch with a different name/asked not to be put onto a branch. I'll send a v3 with this fixed in a bit. > + struct object_id oid; > + struct commit *commit; > + const char *remote; > + > + commit = lookup_commit_reference_by_name(branch); > + if (!commit) > + remote = unique_tracking_name(branch, ); > + if (!commit && remote) { > + opts.new_branch = branch; > + branch = remote; > + } > + } > + > if (opts.new_branch) { > struct child_process cp = CHILD_PROCESS_INIT; > cp.git_cmd = 1; > diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh > index b5c47ac602..e5959800c0 100755 > --- a/t/t2025-worktree-add.sh > +++ b/t/t2025-worktree-add.sh > @@ -6,6 +6,16 @@ test_description='test git worktree add' > > . "$TEST_DIRECTORY"/lib-rebase.sh > > +# Is branch "refs/heads/$1" set to pull from "$2/$3"? > +test_branch_upstream () { > + printf "%s\n" "$2" "refs/heads/$3" >expect.upstream && > + { > + git config "branch.$1.remote" && > + git config "branch.$1.merge" > + } >actual.upstream && > + test_cmp expect.upstream actual.upstream > +} > + > test_expect_success 'setup' ' > test_commit init > ' > @@ -314,4 +324,38 @@ test_expect_success 'rename a branch under bisect not > allowed' ' > test_must_fail git branch -M under-bisect bisect-with-new-name > ' > > +test_expect_success '"add" fails' ' > + test_must_fail git worktree add foo non-existent > +' > + > +test_expect_success '"add" dwims' ' > + test_when_finished rm -rf repo_upstream && > + test_when_finished rm -rf repo_dwim && > + test_when_finished rm -rf foo && > + git init repo_upstream && > + ( > + cd repo_upstream && > + test_commit upstream_master && > + git checkout -b foo && > + test_commit a_foo > + ) && > + git init repo_dwim && > + ( > + cd repo_dwim && > + test_commit dwim_master && > + git remote add repo_upstream ../repo_upstream && > + git config remote.repo_upstream.fetch \ > + "refs/heads/*:refs/remotes/repo_upstream/*" && > + git fetch --all && > + git worktree add ../foo foo > + ) && > + ( > + cd foo && > + test_branch_upstream foo repo_upstream foo && > + git rev-parse repo_upstream/foo >expect && > +
Re: [PATCH] git-send-email: honor $PATH
This patch adds support for PATH, but it also removes the fixed paths. On many systems, unprivileged users don't have /usr/sbin in their PATH, and I know of no systems which provide /usr/lib as a PATH value. Therefore, it's possible that this change will break automatic detection of sendmail for many users. I think what you probably want to do is use entries in PATH first, and leave the two old values as backups at the end. You're perfectly right - forgot about /usr/sbin not in PATH for unprivileged users. Will send a new patch which appends to the original list. signature.asc Description: PGP signature
Re: [PATCH] git-send-email: honor $PATH
On Sat, Nov 18, 2017 at 01:42:49PM +0100, Florian Klink wrote: > This will search $PATH for a sendmail binary, instead of the (previously > fixed) list of paths. > > Signed-off-by: Florian Klink> --- > Documentation/git-send-email.txt | 5 ++--- > git-send-email.perl | 3 ++- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/Documentation/git-send-email.txt > b/Documentation/git-send-email.txt > index bac9014ac..b9b1f2c41 100644 > --- a/Documentation/git-send-email.txt > +++ b/Documentation/git-send-email.txt > @@ -203,9 +203,8 @@ a password is obtained using 'git-credential'. > specify a full pathname of a sendmail-like program instead; > the program must support the `-i` option. Default value can > be specified by the `sendemail.smtpServer` configuration > - option; the built-in default is `/usr/sbin/sendmail` or > - `/usr/lib/sendmail` if such program is available, or > - `localhost` otherwise. > + option; the built-in default is to search in $PATH if such program is > + available, or `localhost` otherwise. This patch adds support for PATH, but it also removes the fixed paths. On many systems, unprivileged users don't have /usr/sbin in their PATH, and I know of no systems which provide /usr/lib as a PATH value. Therefore, it's possible that this change will break automatic detection of sendmail for many users. I think what you probably want to do is use entries in PATH first, and leave the two old values as backups at the end. -- brian m. carlson / brian with sandals: Houston, Texas, US https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH] sha1: add gnutls as a sha1 provider
On Tue, Nov 14, 2017 at 11:47 AM, Todd Zullingerwrote: > > Hi Shawn, > > Shawn Landden wrote: >> >> I think this is preferrable to bringing the assembly routines into the git >> code-base, as a way of getting access to these high-performance routines to >> a git available in Debian, Ubuntu, or Fedora (which all use BLK_SHA1=1 due >> to GPLv2 + OpenSSL license considerations, see Debian Bug #879459). > > > While it seems like it could be useful to have the choice of using the fast > SHA1 implementation without concern about licensing issues, there's a few > details I thought were worth mentioning. > > Fedora moved from OpenSSL SHA1 to BLK_SHA1 to reduce the size of the binaries > and dependencies, not due to licensing issues (Fedora considers OpenSSL a > system library and allows linking GPLv2 code). > > Fedora now uses the default DC_SHA1 (the collision-detecting SHA1 > implementation). DC_SHA1 is not, as far as I know, as fast as the > OpenSSL/GnuTLS SHA1, but it's safer given the increasingly successful attacks > against SHA1. I don't envision changing that to gain performance. (And, of > course, the speed of SHA1 should become less of an issue once git moves to a > new, stronger hash.) > > It looks like the Debian packages use the default DC_SHA1 implementation as > well. Regardless of the licensing concerns regarding OpenSSL in Debian, I > suspect they'll want to use the default, collision-detecting SHA1 > implementation. That doesn't mean a patch to add the option of GnuTLS isn't > useful though. > > Fedora does link with OpenSSL's libcrypto and libssl in Fedora for the > remote-curl helpers and imap-send. I believe the remote-curl helpers just > link with curl, which happens to use OpenSSL on Fedora and could use GnuTLS > instead. The imap-send command might also use curl and whatever crypto > library curl is built with too, but I'm not terribly familiar with imap-send. > (I think those are the only uses of libcrypto or libssl in Fedora's packages, > but I could be mistaken). > > That's a lot of text without having anything to say about the actual patch. > Hopefully it's at least mildly useful to you or others. :) It is all appreciated. I just want to make note that I am still interested in getting this patch in.
Re: [PATCH] config: avoid "write_in_full(fd, buf, len) != len" pattern
Am 18.11.2017 um 18:52 schrieb Jeff King: > On Sat, Nov 18, 2017 at 11:20:04AM +0100, René Scharfe wrote: >> Reported-by: Jeff King>> Signed-off-by: Rene Scharfe > > I'm not sure I deserve a reported-by if I say "it looks fine" but am > totally wrong. ;) Right, wrong -- mere details. You pointed out that there was work to do. :) René
Investissement privé
-- Cet e-mail sollicite strictement votre intérêt pour un partenariat avec moi pour un investissement énorme, répondez pour plus de détails. Cordialement, Khvostova Zhanna
[PATCH 6/6] grep: show non-empty lines before functions with -W
Non-empty lines before a function definition are most likely comments for that function and thus relevant. Include them in function context. Such a non-empty line might also belong to the preceding function if there is no separating blank line. Stop extending the context upwards also at the next function line to make sure only one extra function body is shown at most. Signed-off-by: Rene Scharfe--- grep.c | 27 +++ t/t7810-grep.sh | 2 +- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/grep.c b/grep.c index 2c55d10c55..a69c05edc2 100644 --- a/grep.c +++ b/grep.c @@ -1476,33 +1476,52 @@ static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs, } } +static int is_empty_line(const char *bol, const char *eol); + static void show_pre_context(struct grep_opt *opt, struct grep_source *gs, char *bol, char *end, unsigned lno) { unsigned cur = lno, from = 1, funcname_lno = 0, orig_from; - int funcname_needed = !!opt->funcname; + int funcname_needed = !!opt->funcname, comment_needed = 0; if (opt->pre_context < lno) from = lno - opt->pre_context; if (from <= opt->last_shown) from = opt->last_shown + 1; orig_from = from; - if (opt->funcbody && !match_funcname(opt, gs, bol, end)) { - funcname_needed = 1; + if (opt->funcbody) { + if (match_funcname(opt, gs, bol, end)) + comment_needed = 1; + else + funcname_needed = 1; from = opt->last_shown + 1; } /* Rewind. */ while (bol > gs->buf && cur > from) { + char *next_bol = bol; char *eol = --bol; while (bol > gs->buf && bol[-1] != '\n') bol--; cur--; + if (comment_needed && (is_empty_line(bol, eol) || + match_funcname(opt, gs, bol, eol))) { + comment_needed = 0; + from = orig_from; + if (cur < from) { + cur++; + bol = next_bol; + break; + } + } if (funcname_needed && match_funcname(opt, gs, bol, eol)) { funcname_lno = cur; funcname_needed = 0; - from = orig_from; + if (opt->funcbody) + comment_needed = 1; + else + from = orig_from; } } diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh index 632b905611..c02ca735b9 100755 --- a/t/t7810-grep.sh +++ b/t/t7810-grep.sh @@ -785,7 +785,7 @@ test_expect_success 'grep -W with userdiff' ' git grep -W echo >function-context-userdiff-actual ' -test_expect_failure ' includes preceding comment' ' +test_expect_success ' includes preceding comment' ' grep "# Say hello" function-context-userdiff-actual ' -- 2.15.0
Re: [PATCH v1 2/2] worktree: make add dwim
On 11/15, Thomas Gummerer wrote: > On 11/14, Eric Sunshine wrote: > > On Tue, Nov 14, 2017 at 3:14 PM, Eric Sunshine> > wrote: > > > For my own edification... > > > [...] > > > git worktree add ../topic > > > > > > * Correctly errors out, refusing to create a new branch named "topic", > > > if "topic" is already a branch. > > > > By the way, there's an additional DWIM that could be done here instead > > of erroring out. Specifically, for "git worktree add ../topic": > > > > * If branch "topic" exists, check it out (rather than refusing to > > create a new branch named "topic"). > > I think this would be a good improvement either way as I suspect this > is what users would hope for, and as it currently just dies there are > less backwards compatibility worries. While I still think this would be an improvement, after thinking about it a bit more I think this is somewhat orthogonal to what I'm trying to achieve with this patch series. Therefore I didn't implement this yet, but I'm still thinking of implementing this in a separate topic. > > * If origin/topic exists, DWIM local "topic" branch into existence. > > > > * Otherwise, create new local branch "topic". > > > > > * Creates a new branch named "topic" if no such local branch exists. > > > > > > The desired new DWIMing would change the second bullet point to: > > > > > > * If no local branch named "topic" exists, DWIM it from "origin/topic" > > > if possible, else create a new local branch named "topic".
[PATCH 4/6] t7810: improve check of -W with user-defined function lines
The check for function context (-W) together with user-defined function line patterns reuses hello.c and pretends it's written in a language in which function lines contain either "printf" or a trailing curly brace. That's a bit obscure. Make the test easier to read by adding a small PowerShell script, using a simple, but meaningful expression, and separating out checks for different aspects into dedicated tests instead of simply matching the whole output byte for byte. Also include a test for showing comments before function lines like git diff -W does, which is currently failing because that functionality is not implemented for git grep, yet. Signed-off-by: Rene Scharfe--- t/t7810-grep.sh | 41 +++-- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh index 2a6679c2f5..632b905611 100755 --- a/t/t7810-grep.sh +++ b/t/t7810-grep.sh @@ -60,6 +60,18 @@ test_expect_success setup ' echo " line with leading space3" echo "line without leading space2" } >space && + cat >hello.ps1 <<-\EOF && + # No-op. + function dummy() {} + + # Say hello. + function hello() { + echo "Hello world." + } # hello + + # Still a no-op. + function dummy() {} + EOF git add . && test_tick && git commit -m initial @@ -766,18 +778,27 @@ test_expect_success 'grep -W shows no trailing empty lines' ' test_cmp expected actual ' -cat >expected <.gitattributes && - git grep -W return >actual && - test_cmp expected actual + git config diff.custom.xfuncname "^function .*$" && + echo "hello.ps1 diff=custom" >.gitattributes && + git grep -W echo >function-context-userdiff-actual +' + +test_expect_failure ' includes preceding comment' ' + grep "# Say hello" function-context-userdiff-actual +' + +test_expect_success ' includes function line' ' + grep "=function hello" function-context-userdiff-actual +' + +test_expect_success ' includes matching line' ' + grep ": echo" function-context-userdiff-actual +' + +test_expect_success ' includes last line of the function' ' + grep "} # hello" function-context-userdiff-actual ' for threads in $(test_seq 0 10) -- 2.15.0
[PATCH 2/6] xdiff: factor out is_func_rec()
Add a helper for checking if a given record is a function line. It frees callers from having to deal with the buffer arguments of match_func_rec(). Signed-off-by: Rene Scharfe--- xdiff/xemit.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/xdiff/xemit.c b/xdiff/xemit.c index 6881445e4a..c2d5bd004c 100644 --- a/xdiff/xemit.c +++ b/xdiff/xemit.c @@ -121,6 +121,12 @@ static long match_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri, return xecfg->find_func(rec, len, buf, sz, xecfg->find_func_priv); } +static int is_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri) +{ + char dummy[1]; + return match_func_rec(xdf, xecfg, ri, dummy, sizeof(dummy)) >= 0; +} + struct func_line { long len; char buf[80]; @@ -178,7 +184,6 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, /* Appended chunk? */ if (i1 >= xe->xdf1.nrec) { - char dummy[1]; long i2 = xch->i2; /* @@ -186,8 +191,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, * a whole function was added. */ while (i2 < xe->xdf2.nrec) { - if (match_func_rec(>xdf2, xecfg, i2, - dummy, sizeof(dummy)) >= 0) + if (is_func_rec(>xdf2, xecfg, i2)) goto post_context_calculation; i2++; } -- 2.15.0
[PATCH v2 2/3] worktree: make add dwim
Currently 'git worktree add ', errors out when 'branch' is not a local branch. It has no additional dwim'ing features that one might expect. Make it behave more like 'git checkout ' when the branch doesn't exist locally, but a remote tracking branch uniquely matches the desired branch name, i.e. create a new branch from the remote tracking branch and set the upstream to the remote tracking branch. As 'git worktree add' currently just dies in this situation, there are no backwards compatibility worries when introducing this feature. Signed-off-by: Thomas Gummerer--- Documentation/git-worktree.txt | 7 +++ builtin/worktree.c | 15 ++ t/t2025-worktree-add.sh| 44 ++ 3 files changed, 66 insertions(+) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index b472acc356..3c7c8c3cee 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -52,6 +52,13 @@ is linked to the current repository, sharing everything except working directory specific files such as HEAD, index, etc. `-` may also be specified as ``; it is synonymous with `@{-1}`. + +If is not found but there does exist a tracking branch in +exactly one remote (call it ) with a matching name, treat as +equivalent to + +$ git worktree add -b / + ++ If `` is omitted and neither `-b` nor `-B` nor `--detach` used, then, as a convenience, a new branch based at HEAD is created automatically, as if `-b $(basename )` was specified. diff --git a/builtin/worktree.c b/builtin/worktree.c index 7b9307aa58..92b583ae39 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "checkout.h" #include "config.h" #include "builtin.h" #include "dir.h" @@ -386,6 +387,20 @@ static int add(int ac, const char **av, const char *prefix) opts.new_branch = xstrndup(s, n); } + if (ac == 2) { + struct object_id oid; + struct commit *commit; + const char *remote; + + commit = lookup_commit_reference_by_name(branch); + if (!commit) + remote = unique_tracking_name(branch, ); + if (!commit && remote) { + opts.new_branch = branch; + branch = remote; + } + } + if (opts.new_branch) { struct child_process cp = CHILD_PROCESS_INIT; cp.git_cmd = 1; diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index b5c47ac602..e5959800c0 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -6,6 +6,16 @@ test_description='test git worktree add' . "$TEST_DIRECTORY"/lib-rebase.sh +# Is branch "refs/heads/$1" set to pull from "$2/$3"? +test_branch_upstream () { + printf "%s\n" "$2" "refs/heads/$3" >expect.upstream && + { + git config "branch.$1.remote" && + git config "branch.$1.merge" + } >actual.upstream && + test_cmp expect.upstream actual.upstream +} + test_expect_success 'setup' ' test_commit init ' @@ -314,4 +324,38 @@ test_expect_success 'rename a branch under bisect not allowed' ' test_must_fail git branch -M under-bisect bisect-with-new-name ' +test_expect_success '"add" fails' ' + test_must_fail git worktree add foo non-existent +' + +test_expect_success '"add" dwims' ' + test_when_finished rm -rf repo_upstream && + test_when_finished rm -rf repo_dwim && + test_when_finished rm -rf foo && + git init repo_upstream && + ( + cd repo_upstream && + test_commit upstream_master && + git checkout -b foo && + test_commit a_foo + ) && + git init repo_dwim && + ( + cd repo_dwim && + test_commit dwim_master && + git remote add repo_upstream ../repo_upstream && + git config remote.repo_upstream.fetch \ + "refs/heads/*:refs/remotes/repo_upstream/*" && + git fetch --all && + git worktree add ../foo foo + ) && + ( + cd foo && + test_branch_upstream foo repo_upstream foo && + git rev-parse repo_upstream/foo >expect && + git rev-parse foo >actual && + test_cmp expect actual + ) +' + test_done -- 2.15.0.345.gf926f18f3d
[PATCH v2 1/3] checkout: factor out functions to new lib file
Factor the functions out, so they can be re-used from other places. In particular these functions will be re-used in builtin/worktree.c to make git worktree add dwim more. While there add some docs to the function. Signed-off-by: Thomas Gummerer--- The previous round of this series is at https://public-inbox.org/git/20171112134305.3949-1-t.gumme...@gmail.com/. Thanks Junio and Eric for the comments on the previous round! Makefile | 1 + builtin/checkout.c | 41 + checkout.c | 42 ++ checkout.h | 13 + 4 files changed, 57 insertions(+), 40 deletions(-) create mode 100644 checkout.c create mode 100644 checkout.h diff --git a/Makefile b/Makefile index cd75985991..8d603c7443 100644 --- a/Makefile +++ b/Makefile @@ -757,6 +757,7 @@ LIB_OBJS += branch.o LIB_OBJS += bulk-checkin.o LIB_OBJS += bundle.o LIB_OBJS += cache-tree.o +LIB_OBJS += checkout.o LIB_OBJS += color.o LIB_OBJS += column.o LIB_OBJS += combine-diff.o diff --git a/builtin/checkout.c b/builtin/checkout.c index fc4f8fd2ea..9e1cfd10b3 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1,5 +1,6 @@ #include "builtin.h" #include "config.h" +#include "checkout.h" #include "lockfile.h" #include "parse-options.h" #include "refs.h" @@ -874,46 +875,6 @@ static int git_checkout_config(const char *var, const char *value, void *cb) return git_xmerge_config(var, value, NULL); } -struct tracking_name_data { - /* const */ char *src_ref; - char *dst_ref; - struct object_id *dst_oid; - int unique; -}; - -static int check_tracking_name(struct remote *remote, void *cb_data) -{ - struct tracking_name_data *cb = cb_data; - struct refspec query; - memset(, 0, sizeof(struct refspec)); - query.src = cb->src_ref; - if (remote_find_tracking(remote, ) || - get_oid(query.dst, cb->dst_oid)) { - free(query.dst); - return 0; - } - if (cb->dst_ref) { - free(query.dst); - cb->unique = 0; - return 0; - } - cb->dst_ref = query.dst; - return 0; -} - -static const char *unique_tracking_name(const char *name, struct object_id *oid) -{ - struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 }; - cb_data.src_ref = xstrfmt("refs/heads/%s", name); - cb_data.dst_oid = oid; - for_each_remote(check_tracking_name, _data); - free(cb_data.src_ref); - if (cb_data.unique) - return cb_data.dst_ref; - free(cb_data.dst_ref); - return NULL; -} - static int parse_branchname_arg(int argc, const char **argv, int dwim_new_local_branch_ok, struct branch_info *new, diff --git a/checkout.c b/checkout.c new file mode 100644 index 00..b0c744d37a --- /dev/null +++ b/checkout.c @@ -0,0 +1,42 @@ +#include "cache.h" +#include "remote.h" + +struct tracking_name_data { + /* const */ char *src_ref; + char *dst_ref; + struct object_id *dst_oid; + int unique; +}; + +static int check_tracking_name(struct remote *remote, void *cb_data) +{ + struct tracking_name_data *cb = cb_data; + struct refspec query; + memset(, 0, sizeof(struct refspec)); + query.src = cb->src_ref; + if (remote_find_tracking(remote, ) || + get_oid(query.dst, cb->dst_oid)) { + free(query.dst); + return 0; + } + if (cb->dst_ref) { + free(query.dst); + cb->unique = 0; + return 0; + } + cb->dst_ref = query.dst; + return 0; +} + +const char *unique_tracking_name(const char *name, struct object_id *oid) +{ + struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 }; + cb_data.src_ref = xstrfmt("refs/heads/%s", name); + cb_data.dst_oid = oid; + for_each_remote(check_tracking_name, _data); + free(cb_data.src_ref); + if (cb_data.unique) + return cb_data.dst_ref; + free(cb_data.dst_ref); + return NULL; +} diff --git a/checkout.h b/checkout.h new file mode 100644 index 00..9980711179 --- /dev/null +++ b/checkout.h @@ -0,0 +1,13 @@ +#ifndef CHECKOUT_H +#define CHECKOUT_H + +#include "cache.h" + +/* + * Check if the branch name uniquely matches a branch name on a remote + * tracking branch. Return the name of the remote if such a branch + * exists, NULL otherwise. + */ +extern const char *unique_tracking_name(const char *name, struct object_id *oid); + +#endif /* CHECKOUT_H */ -- 2.15.0.345.gf926f18f3d
[PATCH v2 3/3] worktree: make add dwim
Currently 'git worktree add ' creates a new branch named after the basename of the , that matches the HEAD of whichever worktree we were on when calling "git worktree add ". Make 'git worktree add behave more like the dwim machinery in 'git checkout ', i.e. check if the new branch name uniquely matches the branch name of a remote tracking branch, and if so check out that branch and set the upstream to the remote tracking branch. This is a change of behaviour compared to the current behaviour, where we create a new branch matching HEAD. However as 'git worktree' is still an experimental feature, and it's easy to notice/correct the behaviour in case it's not what the user desired it's probably okay to break existing behaviour here. In order to also satisfy users who want the current behaviour of creating a new branch from HEAD, add a '--no-track' flag, which disables the new behaviour, and keeps the old behaviour of creating a new branch from the head of the current worktree. Signed-off-by: Thomas Gummerer--- I went back and forth between hiding this behing a flag, and making it default and having a flag for getting the old behaviour back. In the end I went for making the new behaviour the default, because the 'worktree' feature is still experimental, and it's easy to correct the behaviour if it's not what was desired. I also think this is the more intuitve behaviour, but clearly I'm biased because *I* want to it to behave this way. I'm happy to keep the old behaviour the default and hide the new behaviour behind a flag if we feel this is too much of a change in behaviour at this point. Documentation/git-worktree.txt | 15 --- builtin/worktree.c | 9 +++ t/t2025-worktree-add.sh| 60 ++ 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 3c7c8c3cee..11cac104d9 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -60,9 +60,18 @@ $ git worktree add -b / + If `` is omitted and neither `-b` nor `-B` nor `--detach` used, -then, as a convenience, a new branch based at HEAD is created automatically, -as if `-b $(basename )` was specified. - +then, as a convenience, if there exists a tracking branch in exactly +one remote (call it ) matching the basename of the path +(call it ), treat it as equivalent to + +$ git worktree add -b / + +If no tracking branch exists in exactly one remote, is +created based on HEAD, as if `-b $(basename )` was specified. ++ +To disable the behaviour of trying to match the basename of to +a remote, and always create a new branch from HEAD, the `--no-track` +flag can be passed to `git worktree add`. list:: List details of each worktree. The main worktree is listed first, followed by diff --git a/builtin/worktree.c b/builtin/worktree.c index 92b583ae39..82088415b8 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -342,6 +342,7 @@ static int add(int ac, const char **av, const char *prefix) const char *new_branch_force = NULL; char *path; const char *branch; + int track_dwim = 1; struct option options[] = { OPT__FORCE(, N_("checkout even if already checked out in other worktree")), OPT_STRING('b', NULL, _branch, N_("branch"), @@ -351,6 +352,7 @@ static int add(int ac, const char **av, const char *prefix) OPT_BOOL(0, "detach", , N_("detach HEAD at named commit")), OPT_BOOL(0, "checkout", , N_("populate the new working tree")), OPT_BOOL(0, "lock", _locked, N_("keep the new working tree locked")), + OPT_BOOL(0, "track", _dwim, N_("checkout upstream branch if there's a unique match")), OPT_END() }; @@ -385,6 +387,13 @@ static int add(int ac, const char **av, const char *prefix) int n; const char *s = worktree_basename(path, ); opts.new_branch = xstrndup(s, n); + if (track_dwim) { + struct object_id oid; + const char *remote = + unique_tracking_name(opts.new_branch, ); + if (remote) + branch = remote; + } } if (ac == 2) { diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index e5959800c0..a566d867fe 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -358,4 +358,64 @@ test_expect_success '"add" dwims' ' ) ' +test_expect_success 'git worktree add --no-track does not set up tracking' ' + test_when_finished rm -rf repo_a && + test_when_finished rm -rf repo_b && + test_when_finished rm -rf foo && + git init repo_a && + ( + cd repo_a && +
[PATCH 5/6] grep: update boundary variable for pre-context
Function context can be bigger than -A/-B/-C context. To find the beginning of the combined context we search backwards. Currently we check at each loop iteration what we're looking for and determine the effective upper boundary based on that. Simplify this a bit by setting the variable "from" to the lowest unshown line number up front if we're looking for a function line and set it back to the required -B/-C context line number when we find one. This prepares the ground for the next patch; no functional change intended. Signed-off-by: Rene Scharfe--- grep.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/grep.c b/grep.c index d0b9b6cdfa..2c55d10c55 100644 --- a/grep.c +++ b/grep.c @@ -1479,20 +1479,21 @@ static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs, static void show_pre_context(struct grep_opt *opt, struct grep_source *gs, char *bol, char *end, unsigned lno) { - unsigned cur = lno, from = 1, funcname_lno = 0; + unsigned cur = lno, from = 1, funcname_lno = 0, orig_from; int funcname_needed = !!opt->funcname; - if (opt->funcbody && !match_funcname(opt, gs, bol, end)) - funcname_needed = 2; - if (opt->pre_context < lno) from = lno - opt->pre_context; if (from <= opt->last_shown) from = opt->last_shown + 1; + orig_from = from; + if (opt->funcbody && !match_funcname(opt, gs, bol, end)) { + funcname_needed = 1; + from = opt->last_shown + 1; + } /* Rewind. */ - while (bol > gs->buf && - cur > (funcname_needed == 2 ? opt->last_shown + 1 : from)) { + while (bol > gs->buf && cur > from) { char *eol = --bol; while (bol > gs->buf && bol[-1] != '\n') @@ -1501,6 +1502,7 @@ static void show_pre_context(struct grep_opt *opt, struct grep_source *gs, if (funcname_needed && match_funcname(opt, gs, bol, eol)) { funcname_lno = cur; funcname_needed = 0; + from = orig_from; } } -- 2.15.0
[PATCH 3/6] xdiff: show non-empty lines before functions with -W
Non-empty lines before a function definition are most likely comments for that function and thus relevant. Include them in function context. Such a non-empty line might also belong to the preceeding function if there is no separating blank line. Stop extending the context upwards also at the next function line to make sure only one extra function body is shown at most. Original-patch-by: Vegard NossumSigned-off-by: Rene Scharfe --- t/t4051-diff-function-context.sh | 2 +- xdiff/xemit.c| 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh index 30fc5bf2b3..2d76a971c4 100755 --- a/t/t4051-diff-function-context.sh +++ b/t/t4051-diff-function-context.sh @@ -85,7 +85,7 @@ test_expect_success 'setup' ' check_diff changed_hello 'changed function' -test_expect_failure ' context includes comment' ' +test_expect_success ' context includes comment' ' grep "^ .*Hello comment" changed_hello.diff ' diff --git a/xdiff/xemit.c b/xdiff/xemit.c index c2d5bd004c..7778dc2b19 100644 --- a/xdiff/xemit.c +++ b/xdiff/xemit.c @@ -204,6 +204,9 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, } fs1 = get_func_line(xe, xecfg, NULL, i1, -1); + while (fs1 > 0 && !is_empty_rec(>xdf1, fs1 - 1) && + !is_func_rec(>xdf1, xecfg, fs1 - 1)) + fs1--; if (fs1 < 0) fs1 = 0; if (fs1 < s1) { -- 2.15.0
[PATCH 1/6] t4051: add test for comments preceding function lines
When showing function context it would be helpful to show comments immediately before declarations, as they are most likely relevant. Add a test for that. Signed-off-by: Rene Scharfe--- t/t4051-diff-function-context.sh | 4 t/t4051/hello.c | 3 +++ 2 files changed, 7 insertions(+) diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh index 3e6b485ecb..30fc5bf2b3 100755 --- a/t/t4051-diff-function-context.sh +++ b/t/t4051-diff-function-context.sh @@ -85,6 +85,10 @@ test_expect_success 'setup' ' check_diff changed_hello 'changed function' +test_expect_failure ' context includes comment' ' + grep "^ .*Hello comment" changed_hello.diff +' + test_expect_success ' context includes begin' ' grep "^ .*Begin of hello" changed_hello.diff ' diff --git a/t/t4051/hello.c b/t/t4051/hello.c index 63b1a1e4ef..73e767e178 100644 --- a/t/t4051/hello.c +++ b/t/t4051/hello.c @@ -1,4 +1,7 @@ +/* + * Hello comment. + */ static void hello(void)// Begin of hello { /* -- 2.15.0
[PATCH 0/6] show non-empty lines before functions with diff/grep -W
The option -W/--function-context lets git diff and git grep show the whole surrounding function as context. For the sake of simplicity and performance they don't fully parse the files, but as a heuristic show all lines from the preceding function line to the next one. This series refines that heuristic and extends the context to include any non-empty lines before the preceding function line as well. They most likely contain comments related to that function and are thus relevant for reviewing diffs and search results. Idea and original implementation for git diff by Vegard Nossum: https://public-inbox.org/git/1484324112-17773-2-git-send-email-vegard.nos...@oracle.com/ t4051: add test for comments preceeding function lines xdiff: factor out is_func_rec() xdiff: show non-empty lines before functions with -W t7810: improve check of -W with user-defined function lines grep: update boundary variable for pre-context grep: show non-empty lines before functions with -W grep.c | 35 +++--- t/t4051-diff-function-context.sh | 4 t/t4051/hello.c | 3 +++ t/t7810-grep.sh | 41 ++-- xdiff/xemit.c| 13 ++--- 5 files changed, 76 insertions(+), 20 deletions(-) -- 2.15.0
Re: [PATCH] config: avoid "write_in_full(fd, buf, len) != len" pattern
On Sat, Nov 18, 2017 at 11:20:04AM +0100, René Scharfe wrote: > Am 17.11.2017 um 23:06 schrieb Jeff King: > > There's one more case in write_section() that uses "==". That's not > > actually wrong, but I wonder if we'd want to make it "< 0" for > > consistency. > > Actually it *is* wrong. Thanks for digging, I didn't look beyond that single line. > -- >8 -- > Subject: [PATCH] config: flip return value of write_section() > > d9bd4cbb9cc (config: flip return value of store_write_*()) made > write_section() follow the convention of write(2) to return -1 on error > and the number of written bytes on success. 3b48045c6c7 (Merge branch > 'sd/branch-copy') changed it back to returning 0 on error and 1 on > success, but left its callers still checking for negative values. > > Let write_section() follow the convention of write(2) again to meet the > expectations of its callers. Yikes. It looks like this slipped by on the tests because we always check "< 0" in the callers, not non-zero. So success would not look like failure, but failure would look like success. And write failure does not happen regularly in the test suite. So this looks correct, and well-explained. > Reported-by: Jeff King> Signed-off-by: Rene Scharfe I'm not sure I deserve a reported-by if I say "it looks fine" but am totally wrong. ;) -Peff
Re: [PATCH] launch_editor(): indicate that Git waits for user input
On Thursday 16 November 2017 11:34 AM, Junio C Hamano wrote: * I tried this with "emacsclient" but it turns out that Emacs folks have already thought about this issue, and the program says "Waiting for Emacs..." while it does its thing, so from that point of view, perhaps as Stefan said originally, the editor Lars had trouble with is what is at fault and needs fixing? I dunno. Also, emacsclient seems to conclude its message, once the editing is done, by emitting LF _before_ we have a chance to do the "go back to the beginning and clear" dance, so it probably is not a very good example to emulate the situation Lars had trouble with. You cannot observe the nuking of the "launched, waiting..." with it. I tried this patch with 'gedit' and it seems to work fine except for one particular case. When the launched editor gets killed somehow when waiting for user input, the error message shown in the terminal seems to be following the "Launched editor..." message immediately, making it hard to identify it. $ GIT_EDITOR=gedit ./git commit --allow-empty Launched your editor, waiting...error: gedit died of signal 15 error: There was a problem with the editor 'gedit'. Please supply the message using either -m or -F option. Though this is not something that's going to happen very often, I'm not sure if this is to be considered. Just wanted to note what I observed. editor.c | 25 + 1 file changed, 25 insertions(+) diff --git a/editor.c b/editor.c index 7519edecdc..1321944716 100644 --- a/editor.c +++ b/editor.c @@ -40,6 +40,28 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en const char *args[] = { editor, real_path(path), NULL }; struct child_process p = CHILD_PROCESS_INIT; int ret, sig; + static const char *close_notice = NULL; Just a little curious, what's the advantage of making 'close_notice' 'static' over 'auto' ? + + if (isatty(2) && !close_notice) { + char *term = getenv("TERM"); + + if (term && strcmp(term, "dumb")) + /* +* go back to the beginning and erase the +* entire line if the terminal is capable +* to do so, to avoid wasting the vertical +* space. +*/ + close_notice = "\r\033[K"; + else + /* otherwise, complete and waste the line */ + close_notice = "done.\n"; + } + + if (close_notice) { + fprintf(stderr, "Launched your editor, waiting..."); + fflush(stderr); Being curious again, is flushing 'stderr' required ? AFAIK, 'stderr' is unbuffered by default and I didn't notice any calls that changed the buffering mode of it along this code path. + } p.argv = args; p.env = env; @@ -53,11 +75,14 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en sig = ret - 128; sigchain_pop(SIGINT); sigchain_pop(SIGQUIT); + if (sig == SIGINT || sig == SIGQUIT) raise(sig); if (ret) return error("There was a problem with the editor '%s'.", editor); + if (close_notice) + fputs(close_notice, stderr); } if (!buffer)
[PATCH 1/4] branch: improve documentation and naming of create_branch() parameters
The documentation for 'create_branch()' was incomplete as it didn't say what certain parameters were used for. Further a parameter name wasn't very communicative. So, add missing documentation for the sake of completeness and easy reference. Also, rename the concerned parameter to make its name more communicative. Signed-off-by: Kaartic Sivaraam--- branch.c | 4 ++-- branch.h | 7 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/branch.c b/branch.c index 62f7b0d8c..3e8d2f93f 100644 --- a/branch.c +++ b/branch.c @@ -228,7 +228,7 @@ N_("\n" "\"git push -u\" to set the upstream config as you push."); void create_branch(const char *name, const char *start_name, - int force, int reflog, int clobber_head, + int force, int reflog, int clobber_head_ok, int quiet, enum branch_track track) { struct commit *commit; @@ -244,7 +244,7 @@ void create_branch(const char *name, const char *start_name, if (validate_new_branchname(name, , force, track == BRANCH_TRACK_OVERRIDE || - clobber_head)) { + clobber_head_ok)) { if (!force) dont_change_ref = 1; else diff --git a/branch.h b/branch.h index b07788558..cb6411f84 100644 --- a/branch.h +++ b/branch.h @@ -15,12 +15,17 @@ * * - reflog creates a reflog for the branch * + * - clobber_head_ok allows the currently checked out (hence existing) + * branch to be overwritten; without 'force', it has no effect. + * + * - quiet suppresses tracking information + * * - track causes the new branch to be configured to merge the remote branch * that start_name is a tracking branch for (if any). */ void create_branch(const char *name, const char *start_name, int force, int reflog, - int clobber_head, int quiet, enum branch_track track); + int clobber_head_ok, int quiet, enum branch_track track); /* * Validates that the requested branch may be created, returning the -- 2.15.0.291.g0d8980c5d
[PATCH 3/4] branch: update warning message shown when copying a misnamed branch
When a user tries to rename a branch that has a "bad name" (e.g., starts with a '-') then we warn them that the misnamed branch has been renamed "away". A similar message is shown when trying to create a copy of a misnamed branch even though it doesn't remove the misnamed branch. This is not correct and may confuse the user. So, update the warning message shown to be more precise that only a copy of the misnamed branch has been created. It's better to show the warning message than not showing it at all as it makes the user aware of the presence of a misnamed branch. Signed-off-by: Kaartic Sivaraam--- builtin/branch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/branch.c b/builtin/branch.c index 4edef5baa..ca9d8abd0 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -507,7 +507,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int if (recovery) { if (copy) - warning(_("Copied a misnamed branch '%s' away"), + warning(_("Created a copy of a misnamed branch '%s'"), oldref.buf + 11); else warning(_("Renamed a misnamed branch '%s' away"), -- 2.15.0.291.g0d8980c5d
[PATCH 2/4] branch: group related arguments of create_branch()
39bd6f726 (Allow checkout -B to update the current branch, 2011-11-26) added 'clobber_head' (now, 'clobber_head_ok') "before" 'track' as 'track' was closely related 'clobber_head' for the purpose the commit wanted to achieve. Looking from the perspective of how the arguments are used it turns out that 'clobber_head' is more related to 'force' than it is to 'track'. So, re-order the arguments to keep the related arguments close to each other. Signed-off-by: Kaartic Sivaraam--- branch.c | 2 +- branch.h | 9 + builtin/branch.c | 2 +- builtin/checkout.c | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/branch.c b/branch.c index 3e8d2f93f..bd607ae97 100644 --- a/branch.c +++ b/branch.c @@ -228,7 +228,7 @@ N_("\n" "\"git push -u\" to set the upstream config as you push."); void create_branch(const char *name, const char *start_name, - int force, int reflog, int clobber_head_ok, + int force, int clobber_head_ok, int reflog, int quiet, enum branch_track track) { struct commit *commit; diff --git a/branch.h b/branch.h index cb6411f84..f66536a10 100644 --- a/branch.h +++ b/branch.h @@ -13,19 +13,20 @@ * * - force enables overwriting an existing (non-head) branch * - * - reflog creates a reflog for the branch - * * - clobber_head_ok allows the currently checked out (hence existing) * branch to be overwritten; without 'force', it has no effect. * + * - reflog creates a reflog for the branch + * * - quiet suppresses tracking information * * - track causes the new branch to be configured to merge the remote branch * that start_name is a tracking branch for (if any). + * */ void create_branch(const char *name, const char *start_name, - int force, int reflog, - int clobber_head_ok, int quiet, enum branch_track track); + int force, int clobber_head_ok, + int reflog, int quiet, enum branch_track track); /* * Validates that the requested branch may be created, returning the diff --git a/builtin/branch.c b/builtin/branch.c index 33fd5fcfd..4edef5baa 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -806,7 +806,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix) die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead.")); create_branch(argv[0], (argc == 2) ? argv[1] : head, - force, reflog, 0, quiet, track); + force, 0, reflog, quiet, track); } else usage_with_options(builtin_branch_usage, options); diff --git a/builtin/checkout.c b/builtin/checkout.c index 7d8bcc383..6068f8d8c 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -640,8 +640,8 @@ static void update_refs_for_switch(const struct checkout_opts *opts, else create_branch(opts->new_branch, new->name, opts->new_branch_force ? 1 : 0, - opts->new_branch_log, opts->new_branch_force ? 1 : 0, + opts->new_branch_log, opts->quiet, opts->track); new->name = opts->new_branch; -- 2.15.0.291.g0d8980c5d
[PATCH 4/4] builtin/branch: strip refs/heads/ using skip_prefix
Instead of hard-coding the offset strlen("refs/heads/") to skip the prefix "refs/heads/" use the skip_prefix() function which is more communicative and verifies that the string actually starts with that prefix. Though we don't check for the result of verification here as it's (almost) always the case that the string does start with "refs/heads", it's just better to avoid hard-coding and be more communicative. Signed-off-by: Kaartic Sivaraam--- builtin/branch.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index ca9d8abd0..8c546a958 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -462,6 +462,8 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int { struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT; struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT; + const char *prefix_free_oldref = NULL; + const char *prefix_free_newref = NULL; int recovery = 0; int clobber_head_ok; @@ -493,13 +495,17 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int reject_rebase_or_bisect_branch(oldref.buf); + /* At this point it should be safe to believe that the refs have the + prefix "refs/heads" */ + skip_prefix(oldref.buf, "refs/heads/", _free_oldref); + skip_prefix(newref.buf, "refs/heads/", _free_newref); + if (copy) strbuf_addf(, "Branch: copied %s to %s", oldref.buf, newref.buf); else strbuf_addf(, "Branch: renamed %s to %s", oldref.buf, newref.buf); - if (!copy && rename_ref(oldref.buf, newref.buf, logmsg.buf)) die(_("Branch rename failed")); if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf)) @@ -508,10 +514,10 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int if (recovery) { if (copy) warning(_("Created a copy of a misnamed branch '%s'"), - oldref.buf + 11); + prefix_free_oldref); else warning(_("Renamed a misnamed branch '%s' away"), - oldref.buf + 11); + prefix_free_oldref); } if (!copy && @@ -520,9 +526,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int strbuf_release(); - strbuf_addf(, "branch.%s", oldref.buf + 11); + strbuf_addf(, "branch.%s", prefix_free_oldref); strbuf_release(); - strbuf_addf(, "branch.%s", newref.buf + 11); + strbuf_addf(, "branch.%s", prefix_free_newref); strbuf_release(); if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0) die(_("Branch is renamed, but update of config-file failed")); -- 2.15.0.291.g0d8980c5d
[PATCH 0/4] cleanups surrounding branch
On the process of making 'git' give more useful error messages when trying to rename a branch[0], I found a few things that could be cleaned up. After noticing that the cleanup commits exceeded the commits that are related to the series, I thought it would be better to separate the cleanups into an independent series to keep that topic focused on improving the error messages. 1/4 and 2/4 were part of that series until v3. The others are new cleanups. This series goes on top of 'master' and can be found in my fork as branch 'work/branch-cleanups'[1]. Note: 1/4 might possibly have some conflicts with jc/branch-name-sanity Footnotes: [0]: cf. <20171102065407.25404-1-kaartic.sivar...@gmail.com> [1]: https://github.com/sivaraam/git/tree/work/branch-cleanups Kaartic Sivaraam (4): branch: improve documentation and naming of create_branch() parameters branch: group related arguments of create_branch() branch: update warning message shown when copying a misnamed branch builtin/branch: strip refs/heads/ using skip_prefix branch.c | 4 ++-- branch.h | 10 -- builtin/branch.c | 20 +--- builtin/checkout.c | 2 +- 4 files changed, 24 insertions(+), 12 deletions(-) -- 2.15.0.291.g0d8980c5d
[PATCH] git-rebase: clean up dashed-usages in messages
Signed-off-by: Kaartic Sivaraam--- git-rebase.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index 6344e8d5e..42a485aaa 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -9,7 +9,7 @@ OPTIONS_STUCKLONG=t OPTIONS_SPEC="\ git rebase [-i] [options] [--exec ] [--onto ] [] [] git rebase [-i] [options] [--exec ] [--onto ] --root [] -git-rebase --continue | --abort | --skip | --edit-todo +git rebase --continue | --abort | --skip | --edit-todo -- Available options are v,verbose! display a diffstat of what changed upstream @@ -216,7 +216,7 @@ run_pre_rebase_hook () { } test -f "$apply_dir"/applying && - die "$(gettext "It looks like git-am is in progress. Cannot rebase.")" + die "$(gettext "It looks like you are in the middle of an am session. Cannot rebase.")" if test -d "$apply_dir" then -- 2.15.0.291.g0d8980c5d
Re: [PATCH v3 0/8] sequencer: don't fork git commit
On 18/11/17 03:57, Junio C Hamano wrote: > Junio C Hamanowrites: > >> Phillip Wood writes: >> >>> From: Phillip Wood >>> >>> I've updated these based on the feedback for v2. I've dropped the >>> patch that stopped print_commit_summary() from dying as I think it is >>> better to die than return an error (see the commit message of the >>> patch that adds print_commit_summary() for the reasoning). Apart from >>> that they're minor changes - style fixes and a reworded a commit message. >> >> Thanks for further polishing this topic; I found nothing in the >> update that was questionable. Will replace. >> >> With this, perhaps it is ready for 'next'? > > Not really. I needed at least this to get it even compile, which > hints that I do not yet know what _else_ I missed by skimming this > round of the series. My apologies, it seems that I was half alseep yesterday morning and in my haste to update these patches I wasn't paying proper attention to what I was doing. For some reason after I dropped the patch that converted print_commit_summary() to return an error rather than die I forgot to amend the last patch to match and then compounded the mistake by forgetting to compile and test properly before sending them. I think they're okay now but I'll double check the changes before sending again in case there are any other embarrassing mistakes lurking. As the white rabbit said "The hurrier I go the behinder I get" Phillip > sequencer.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/sequencer.c b/sequencer.c > index 37460db6b1..63cfb6ddd9 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1139,8 +1139,8 @@ static int do_commit(const char *msg_file, const char > *author, > unlink(git_path_cherry_pick_head()); > unlink(git_path_merge_msg()); > if (!is_rebase_i(opts)) > - res = print_commit_summary(NULL, , > - SUMMARY_SHOW_AUTHOR_DATE); > + print_commit_summary(NULL, , > + SUMMARY_SHOW_AUTHOR_DATE); > return res; > } > } >
[PATCH] git-send-email: honor $PATH
This will search $PATH for a sendmail binary, instead of the (previously fixed) list of paths. Signed-off-by: Florian Klink--- Documentation/git-send-email.txt | 5 ++--- git-send-email.perl | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index bac9014ac..b9b1f2c41 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -203,9 +203,8 @@ a password is obtained using 'git-credential'. specify a full pathname of a sendmail-like program instead; the program must support the `-i` option. Default value can be specified by the `sendemail.smtpServer` configuration - option; the built-in default is `/usr/sbin/sendmail` or - `/usr/lib/sendmail` if such program is available, or - `localhost` otherwise. + option; the built-in default is to search in $PATH if such program is + available, or `localhost` otherwise. --smtp-server-port=:: Specifies a port different from the default port (SMTP diff --git a/git-send-email.perl b/git-send-email.perl index 2208dcc21..8e357aeab 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -885,7 +885,8 @@ if (defined $initial_reply_to) { } if (!defined $smtp_server) { - foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) { + my @sendmail_paths = map {"$_/sendmail"} split /:/, $ENV{PATH}; + foreach (@sendmail_paths) { if (-x $_) { $smtp_server = $_; last; -- 2.15.0
Re: Is it not bug git stash -- does not work at non-root directory?
小川恭史writes: > I upgraded the version of git from 2.13.1 to 2.15.0 on Mac and fixed my issue. > Thanks. Ah, yes, that bug was fixed in the 2.14.0 timeframe but was backported to 2.13.2 and onwards (it was a bug in 2.13.0, I think).
Re: [PATCH v3 0/8] sequencer: don't fork git commit
On 18/11/17 03:57, Junio C Hamano wrote: > Junio C Hamanowrites: > >> Phillip Wood writes: >> >>> From: Phillip Wood >>> >>> I've updated these based on the feedback for v2. I've dropped the >>> patch that stopped print_commit_summary() from dying as I think it is >>> better to die than return an error (see the commit message of the >>> patch that adds print_commit_summary() for the reasoning). Apart from >>> that they're minor changes - style fixes and a reworded a commit message. >> >> Thanks for further polishing this topic; I found nothing in the >> update that was questionable. Will replace. >> >> With this, perhaps it is ready for 'next'? > > Not really. I needed at least this to get it even compile, which > hints that I do not yet know what _else_ I missed by skimming this > round of the series. Sorry I'm not sure what happened there, by branch has the missing 'res = ' something must have happened to the patches. I'll sort it out and resend Phillip > sequencer.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/sequencer.c b/sequencer.c > index 37460db6b1..63cfb6ddd9 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1139,8 +1139,8 @@ static int do_commit(const char *msg_file, const char > *author, > unlink(git_path_cherry_pick_head()); > unlink(git_path_merge_msg()); > if (!is_rebase_i(opts)) > - res = print_commit_summary(NULL, , > - SUMMARY_SHOW_AUTHOR_DATE); > + print_commit_summary(NULL, , > + SUMMARY_SHOW_AUTHOR_DATE); > return res; > } > } >
Re: [PATCH] config: avoid "write_in_full(fd, buf, len) != len" pattern
On 17/11/17 22:06, Jeff King wrote: > On Wed, Nov 15, 2017 at 12:40:43PM +, Phillip Wood wrote: > >> From: Phillip Wood>> >> As explained in commit 06f46f237 (avoid "write_in_full(fd, buf, len) >> != len" pattern, 2017–09–13) the return value of write_in_full() is >> either -1 or the requested number of bytes. As such comparing the >> return value to an unsigned value such as strbuf.len will fail to >> catch errors. Change the code to use the preferred '< 0' check. > > Thanks for catching this. I wondered at first how I missed these obvious > cases, but the answer is that they were added after my commit. :) > > There's one more case in write_section() that uses "==". That's not > actually wrong, but I wonder if we'd want to make it "< 0" for > consistency. Yes, I noticed that but didn't get round to looking at it properly the other day. Rene's fix looks good to me. Best Wishes Phillip
Re: [PATCH] config: avoid "write_in_full(fd, buf, len) != len" pattern
Am 17.11.2017 um 23:06 schrieb Jeff King: > There's one more case in write_section() that uses "==". That's not > actually wrong, but I wonder if we'd want to make it "< 0" for > consistency. Actually it *is* wrong. -- >8 -- Subject: [PATCH] config: flip return value of write_section() d9bd4cbb9cc (config: flip return value of store_write_*()) made write_section() follow the convention of write(2) to return -1 on error and the number of written bytes on success. 3b48045c6c7 (Merge branch 'sd/branch-copy') changed it back to returning 0 on error and 1 on success, but left its callers still checking for negative values. Let write_section() follow the convention of write(2) again to meet the expectations of its callers. Reported-by: Jeff KingSigned-off-by: Rene Scharfe --- config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.c b/config.c index 903abf9533..3f079c77ad 100644 --- a/config.c +++ b/config.c @@ -2315,7 +2315,7 @@ static ssize_t write_section(int fd, const char *key) struct strbuf sb = store_create_section(key); ssize_t ret; - ret = write_in_full(fd, sb.buf, sb.len) == sb.len; + ret = write_in_full(fd, sb.buf, sb.len); strbuf_release(); return ret; -- 2.15.0
Re: Is it not bug git stash -- does not work at non-root directory?
I upgraded the version of git from 2.13.1 to 2.15.0 on Mac and fixed my issue. Thanks. 2017-11-18 16:56 GMT+09:00 Junio C Hamano: > 小川恭史 writes: > >>> Please make it a habit (not limited to when interacting with >>> _this_ project) to state a bit more than "does not work"; >>> instead, say "it is expected to do X, but instead it does Y, and >>> the difference between X and Y I perceive is Z". >> >> Thanks. I'll rewrite the issue. >> >> Assuming that we have sub/something and something is not included anywhere >> else, >> >> cd sub && git stash -- something >> >> is expected to make a stash for sub/something but instead returns error like >> >> error: pathspec 'something' did not match any file(s) known to git. >> Did you forget to 'git add'? >> >> . >> >> I don't know what I should write about 'the difference between X and Y is Z'. > > If the difference between X and Y is obvious there is no need. > > I just tried it and I do not see the command is broken in the way > you describe. > > Trial #1 -- the command fully spelled out. > > $ git.git/master: cd Documentation > $ Documentation/master: echo >>Makefile > $ Documentation/master: git stash push -m "doc-make" -- Makefile > Saved working directory and index state On master: doc-make > $ Documentation/master: git stash show --stat > Documentation/Makefile | 1 + > 1 file changed, 1 insertion(+: > > Trial #2 -- lazily issue the command without subcommand. > > $ git.git/master: cd Documentation > $ Documentation/master: echo >>Makefile > $ Documentation/master: git stash -- Makefile > Saved working directory and index state WIP on master: 89ea799ffc Sync > with maint > $ Documentation/master: git stash show --stat > Documentation/Makefile | 1 + > 1 file changed, 1 insertion(+: > > Trial #3 -- make sure having files with the same name is not hiding any bug. > > $ git.git/master: cd Documentation > $ Documentation/master: echo >>CodingGuidelines > $ Documentation/master: git stash -- CodingGuidelines > Saved working directory and index state WIP on master: 89ea799ffc > $ Documentation/master: git stash show --stat > Documentation/CodingGuidelines | 1 + > 1 file changed, 1 insertion(+) > > Trial #4 -- simulate a PEBKAC > > $ git.git/master: cd Documentation > $ Documentation/master: echo >>no-such-file > $ Documentation/master: git stash -- no-such-file > error: pathspec 'Documentation/no-such-file' did not match any file(s) > known to git. > Did you forget to 'git add'? > > The last one is an expected result---the pathspec given to the > command does not match anything tracked, so without first adding the > file, there is nothing for the command to do. >
Re: Improved error handling (Was: [PATCH 1/2] sequencer: factor out rewrite_file())
Am 17.11.2017 um 23:33 schrieb Jeff King: On Mon, Nov 06, 2017 at 05:13:15PM +0100, Simon Ruderich wrote: On Sat, Nov 04, 2017 at 10:07:00PM -0400, Jeff King wrote: Yes, I think what you've written here (and below) is quite close to the error_context patches I linked elsewhere in the thread. In other words, I think it's a sane approach. In contrast to error_context I'd like to keep all exiting behavior (die, ignore, etc.) in the hand of the caller and not use any callbacks as that makes the control flow much harder to follow. Yeah, I have mixed feelings on that. I think it does make the control flow less clear. At the same time, what I found was that handlers like die/ignore/warn were the thing that gave the most reduction in complexity in the callers. Would you not consider switching over to C++? With exceptions, you get the error context without cluttering the API. (Did I mention that librarification would become a breeze? Do not die in library routines: not a problem anymore, just catch the exception. die_on_error parameters? Not needed anymore. Not to mention that resource leaks would be much, MUCH simpler to treat.) -- Hannes
Hello Beautiful,
Good day dear, i hope this mail meets you well? my name is Jack, from the U.S. I know this may seem inappropriate so i ask for your forgiveness but i wish to get to know you better, if I may be so bold. I consider myself an easy-going man, adventurous, honest and fun loving person but I am currently looking for a relationship in which I will feel loved. I promise to answer any question that you may want to ask me...all i need is just your attention and the chance to know you more. Please tell me more about yourself, if you do not mind. Hope to hear back from you soon. Jack.
hi
-- I am Mrs Nicole i have a pending project of fulfillment to put in your hand, i will need your support to make this ream come through, could you le me know your interest to enable me give you further information, and I hereby advice that you send the below mentioned information I decided to will/donate the sum of 2.7 million Euro to you for the good work of god, and also to help the motherless and less privilege and also forth assistance of the widows. At the moment I cannot take any telephone calls right now due to the fact that my relatives (that have squandered the funds agave them for this purpose before) are around me and my health status also. I have adjusted my will and my lawyer is aware. I have willed those properties to you by quoting my personal file routing and account information. And I have also notified the bank that I am willing that properties to you for a good, effective and prudent work. I know I don't know you but I have been directed to do this by god.ok Please contact this woman for more details you might not get me on line in time contact this Your full name.. Your private telephone number.. Your passport or identity card Your country... ... Your occupation.. Thank you as i wait your reply. Yoursfaithful friand, Mrs. Nicole Maoris --