On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
wrote:
> We transform various object IDs into all-zero object IDs for comparison.
> Adjust the length as well so that this works for all hash algorithms.
>
> Signed-off-by: brian m. carlson
> ---
> diff --git a/t/t0027-auto-crlf.sh
On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
wrote:
> Adjust the test so that it computes variables for object IDs instead of
> using hard-coded hashes.
Nit: s/hashes/hash values/
> Signed-off-by: brian m. carlson
> ---
> diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
> @@ -92,11
On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
wrote:
> Test t tests the "basics of the basics" and as such, checks that we
> have various fixed hard-coded object IDs. The tests relying on these
> assertions have been marked with the SHA1 prerequisite, as they will
> obviously not function
On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
wrote:
> If the hash we're using is 32 bytes in size, attempting to insert a
> 20-byte object name won't work. Since these are synthesized objects
> that are almost all zeros, look them up in a translation table.
>
> Signed-off-by: brian m.
On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
wrote:
> diff --git a/t/oid-info/oid b/t/oid-info/oid
> @@ -0,0 +1,29 @@
> +# All zeros or Fs missing one or two hex segments.
> +zero_1 sha1:000
> +zero_2
>
On Fri, Aug 17, 2018 at 5:17 AM Michał Górny wrote:
> Fix signature_check_clear() to free only values that are non-NULL. This
> especially applies to 'key' and 'signer' members that can be NULL during
> normal operations, depending on exact GnuPG output. While at it, also
> allow other members
On Thu, Aug 16, 2018 at 4:06 AM wrote:
> Add support for configuring default sort ordering for git branches. Command
> line option will override this configured value, using the exact same
> syntax.
>
> Signed-off-by: Samuel Maftoul
> ---
> diff --git a/Documentation/config.txt
On Wed, Aug 15, 2018 at 4:56 PM Elia Pinto wrote:
> Add the '--quiet' option to git worktree,
> as for the other git commands. 'add' is the
> only command affected by it since all other
> commands, except 'list', are currently
> silent by default.
Nit: wrap the commit message at around 70
On Thu, Aug 16, 2018 at 12:41 AM Martin Ågren wrote:
> On Wed, 15 Aug 2018 at 22:56, Elia Pinto wrote:
> The word "currently" means I can't shake the feeling that Eric has a
> very good point in [1]:
>
> It might make sense to say instead that this is adding a --quiet
> option _in general_,
x=$(echo foo &&
...
The closing ")" is already correctly recognized when cuddled or not.
Signed-off-by: Eric Sunshine
---
t/chainlint.sed | 2 +-
.../multi-line-nested-command-substitution.expect | 11 ++-
.../multi-line-nested-
addressed, turn this rather
pathological bit of shell coding into a chainlint test case.
Signed-off-by: Eric Sunshine
---
t/chainlint/t7900-subtree.expect | 10 ++
t/chainlint/t7900-subtree.test | 22 ++
2 files changed, 32 insertions(+)
create mode 100644 t/chainlin
caped quotes are not handled, but support may be added later.)
The original multi-line string recognizer rather cavalierly threw away
all but the final quote, whereas the new one is careful to retain all
quotes, so the "expected" output of a couple existing chainlint tests is
updated t
A here-doc tag can be quoted ('EOF'/"EOF") or escaped (\EOF) to suppress
interpolation within the body. Although, chainlint recognizes escaped
tags, it does not know about quoted tags. For completeness, teach it to
recognize quoted tags, as well.
Signed-off-by: Eric Sunshine
---
t/cha
here-doc:
x=$(cat <<-\END &&
data
END
echo "x")
among others.
Signed-off-by: Eric Sunshine
---
t/chainlint.sed | 7 ---
t/chainlint/here-doc-close-subshell.expect | 2 ++
t/chainlint/here-doc
fool the linter.
Nevertheless, the situation is not ideal since someone writing new
tests, and choosing a name not in the "blessed" set could potentially
trigger a false-positive.
To address this shortcoming, upgrade chainlint.sed to handle arbitrary
here-doc tag names, both at the t
below.
[1]:
https://public-inbox.org/git/20180813084739.16134-1-sunsh...@sunshineco.com/
[2]:
https://public-inbox.org/git/20180730181356.ga156...@aiede.svl.corp.google.com/
Eric Sunshine (6):
chainlint: match arbitrary here-docs tags rather than hard-coded names
chainlint: match quoted her
On Wed, Aug 15, 2018 at 7:16 AM wrote:
> Add support for configuring default sort ordering for git branches. Command
> line option will override this configured value, using the exact same
> syntax.
Your Signed-off-by: is missing. See Documentation/SubmittingPatches.
> diff --git
On Wed, Aug 15, 2018 at 10:33 AM Johannes Schindelin via GitGitGadget
wrote:
> The `chainlint` target compares actual output to expected output, where
> the actual output is generated from files that are specifically checked
> out with LF-only line endings. So the expected output needs to be
>
On Mon, Aug 13, 2018 at 3:27 PM Junio C Hamano wrote:
> Eric Sunshine writes:
>
> > A here-doc tag can be quoted ('EOF') or escaped (\EOF) to suppress
> > interpolation within the body. Although, chainlint recognizes escaped
> > tags, it does not know about quoted tags.
addressed, turn this rather
pathological bit of shell coding into a chainlint test case.
Signed-off-by: Eric Sunshine
---
t/chainlint/t7900-subtree.expect | 10 ++
t/chainlint/t7900-subtree.test | 22 ++
2 files changed, 32 insertions(+)
create mode 100644 t/chainlin
here-doc:
x=$(cat <<-\END &&
data
END
echo "x")
among others.
Signed-off-by: Eric Sunshine
---
t/chainlint.sed | 7 ---
t/chainlint/here-doc-close-subshell.expect | 2 ++
t/chainlint/here-doc
x=$(echo foo &&
...
The closing ")" is already correctly recognized when cuddled or not.
Signed-off-by: Eric Sunshine
---
t/chainlint.sed | 2 +-
.../multi-line-nested-command-substitution.expect | 11 ++-
.../multi-line-nested-
A here-doc tag can be quoted ('EOF') or escaped (\EOF) to suppress
interpolation within the body. Although, chainlint recognizes escaped
tags, it does not know about quoted tags. For completeness, teach it to
recognize quoted tags, as well.
Signed-off-by: Eric Sunshine
---
t/chainlint.sed
caped quotes are not handled, but support may be added later.)
The original multi-line string recognizer rather cavalierly threw away
all but the final quote, whereas the new one is careful to retain all
quotes, so the "expected" output of a couple existing chainlint tests is
updated t
names (in addition to \escaped)
Patch 2/6 is new. Range-diff below.
[1]:
https://public-inbox.org/git/20180807082135.60913-1-sunsh...@sunshineco.com/
[2]:
https://public-inbox.org/git/20180730181356.ga156...@aiede.svl.corp.google.com/
Eric Sunshine (6):
chainlint: match arbitrary here-docs tags
fool the linter.
Nevertheless, the situation is not ideal since someone writing new
tests, and choosing a name not in the "blessed" set could potentially
trigger a false-positive.
To address this shortcoming, upgrade chainlint.sed to handle arbitrary
here-doc tag names, both at the t
On Sun, Aug 12, 2018 at 2:33 AM William Chargin wrote:
> > This is an abuse of test_must_fail() which is intended strictly for
> > testing 'git' invocations which might fail for reasons other than the
> > expected one (for instance, git might crash).
>
> Interesting. I didn't infer this from the
On Sun, Aug 12, 2018 at 12:07 AM William Chargin wrote:
> While the `test_dir_is_empty` function appears correct in most normal
> use cases, it can fail when filenames contain newlines. This patch
> changes the implementation to check that the output of `ls -a` has at
> most two lines (for `.`
On Fri, Aug 10, 2018 at 5:12 PM Johannes Schindelin
wrote:
> On Mon, 30 Jul 2018, Eric Sunshine wrote:
> > I think you can attain the desired behavior by making a final
> > parse_options() call with empty 'options' list after the call to
> > diff_setup_done(). It's prett
On Wed, Aug 8, 2018 at 6:50 PM Jeff King wrote:
> On Tue, Aug 07, 2018 at 04:21:31AM -0400, Eric Sunshine wrote:
> > +# Swallowing here-docs with arbitrary tags requires a bit of finesse. When
> > a
> > +# line such as "cat <out" is seen, the here-
On Tue, Aug 7, 2018 at 5:35 AM Phillip Wood wrote:
> I've updated these based on Eric's suggestions, hopefully they're good
> to go now. Thanks Eric for you help.
Thanks, I left a few comments on patch 2/2. Aside from the '>' vs.
'>=' issue (over which I lost more than a few minutes cogitating),
This changed the date and potentially the author of the commit
> which corrupted the author data compared to its expected value.
>
> Helped-by: Eric Sunshine
> Signed-off-by: Phillip Wood
> ---
> changes since v3:
> - Implemented the simpler scheme suggested by Eric
T
On Tue, Aug 7, 2018 at 5:35 AM Phillip Wood wrote:
> Single quotes should be escaped as \' not \\'. The bad quoting breaks
> the interactive version of 'rebase --root' (which is used when there
> is no '--onto' even if the user does not specify --interactive) for
> authors that contain "'" as
On Tue, Aug 7, 2018 at 9:54 AM Phillip Wood wrote:
> On 07/08/18 11:23, Eric Sunshine wrote:
> > On Tue, Aug 7, 2018 at 5:35 AM Phillip Wood
> > wrote:
> >> + if (n > 0 && s[n] != '\'')
> >> + return 1;
> >
>
On Tue, Aug 7, 2018 at 11:59 PM Brady Trainor wrote:
> If I am reading the git book or manual (https://git-scm.com/), and zoom
> in, and/or have browser sized to a fraction of the screen, I cannot see
> all the text, and have to horizontally scroll back and forth to read at
> that zoom.
>
> Can
Thanks for the submission. A few comments below...
On Wed, Aug 8, 2018 at 2:48 AM Nicholas Guriev wrote:
> This short option saves the number of keys to press for the
> typically git-status command.
> ---
Your sign-off is missing. See Documentation/SubmittingPatches.
It's clear that a short
On Tue, Aug 7, 2018 at 5:09 AM Eric Sunshine wrote:
> On Mon, Aug 6, 2018 at 9:15 AM Johannes Schindelin
> wrote:
> > I think Andrei's assessment is wrong. The code could not test for that
> > earlier, as it did allow ranges to become "abutting" in the process, by
&
On Tue, Aug 7, 2018 at 9:54 AM Andrei Rybak wrote:
> line-log.[ch] use left-closed, right-open interval logic. Change comment
> and debug output to square brackets+parentheses notation to help
> developers avoid off-by-one errors.
> ---
This seems sensible. There might be some reviewers who
(cc:+Karen Arutyunov[1]; perhaps also do so when you re-roll)
In addition to the good review comments by Martin and Duy...
On Tue, Aug 7, 2018 at 9:21 AM Elia Pinto wrote:
> Add the '--quiet' option to git worktree add,
> as for the other git commands.
It might make sense to say instead that
On Tue, Aug 7, 2018 at 5:35 AM Phillip Wood wrote:
> - Reverted the implementation to v2 with more robust detection of the
>missing "'" on the last line of the author script based on a
>suggestion by Eric. This means that this series needs to progress
>closely with
On Mon, Aug 6, 2018 at 9:15 AM Johannes Schindelin
wrote:
> On Sun, 5 Aug 2018, Eric Sunshine wrote:
> > Although this appears to be a faithful translation of the assert() to
> > BUG(), as mentioned by Andrei in his review of 3/4, the existing
> > assert() seems to have an
caped quotes are not handled, but support may be added later.)
The original multi-line string recognizer rather cavalierly threw away
all but the final quote, whereas the new one is careful to retain all
quotes, so the "expected" output of a couple existing chainlint tests is
updated t
-inbox.org/git/CAPig+cRTgh6DStUdmXqvhbL_7sQY6wu21h27rjq_i=kz_d+...@mail.gmail.com/
Eric Sunshine (5):
chainlint: match arbitrary here-docs tags rather than hard-coded names
chainlint: recognize multi-line $(...) when command cuddled with "$("
chainlint: let here-doc and multi-line string com
addressed, turn this rather
pathological bit of shell coding into a chainlint test case.
Signed-off-by: Eric Sunshine
---
t/chainlint/t7900-subtree.expect | 10 ++
t/chainlint/t7900-subtree.test | 22 ++
2 files changed, 32 insertions(+)
create mode 100644 t/chainlin
x=$(echo foo &&
...
The closing ")" is already correctly recognized when cuddled or not.
Signed-off-by: Eric Sunshine
---
t/chainlint.sed | 2 +-
.../multi-line-nested-command-substitution.expect | 11 ++-
.../multi-line-nested-
here-doc:
x=$(cat <<-\END &&
data
END
echo "x")
among others.
Signed-off-by: Eric Sunshine
---
t/chainlint.sed | 7 ---
t/chainlint/here-doc-close-subshell.expect | 2 ++
t/chainlint/here-doc
fool the linter.
Nevertheless, the situation is not ideal since someone writing new
tests, and choosing a name not in the "blessed" set could potentially
trigger a false-positive.
To address this shortcoming, upgrade chainlint.sed to handle arbitrary
here-doc tag names, both at the t
On Mon, Aug 6, 2018 at 11:47 PM Eric Sunshine wrote:
> On Mon, Aug 6, 2018 at 6:36 PM Junio C Hamano wrote:
> > * pw/rebase-i-author-script-fix (2018-08-02) 2 commits
> > - sequencer: fix quoting in write_author_script
> > - sequencer: handle errors in read_author_iden
On Mon, Aug 6, 2018 at 6:36 PM Junio C Hamano wrote:
> * pw/rebase-i-author-script-fix (2018-08-02) 2 commits
> - sequencer: fix quoting in write_author_script
> - sequencer: handle errors in read_author_ident()
> (this branch uses es/rebase-i-author-script-fix.)
>
> Recent "git rebase -i"
On Mon, Aug 6, 2018 at 9:20 PM Hilco Wijbenga wrote:
> But your suggestion did make me think about what behaviour I would
> like to see, exactly. I like that Git removes commits that no longer
> serve any purpose (because I've included their changes in earlier
> commits). So I would not want to
On Mon, Aug 6, 2018 at 1:45 PM Han-Wen Nienhuys wrote:
> The colorization is controlled with the config setting "color.remote".
> [...]
> Helped-by: Duy Nguyen
> Signed-off-by: Han-Wen Nienhuys
> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -1263,6 +1263,18 @@
On Mon, Aug 6, 2018 at 9:02 AM Jeff King wrote:
> On Sun, Aug 05, 2018 at 04:52:31PM -0400, Jeff King wrote:
> > Perhaps even simpler:
> >
> > test "$1" = "$(find "$1")"
>
> Actually, I guess it needs to add "-print", since IIRC that is not the
> default on some old versions of "find".
You
On Sun, Aug 5, 2018 at 11:01 AM Thomas Gummerer wrote:
> On 08/05, Karen Arutyunov wrote:
> > What's quite inconvenient is that the 'git worktree add' command prints some
> > output by default and there is no way to suppress it, as it normally can be
> > achieved with the --quiet option for the
On Sun, Aug 5, 2018 at 6:14 AM Eric Sunshine wrote:
> Having said that, a much easier fix is to use
> range_set_append_unsafe() here, and then at the bottom of the loop,
> invoke 'sort_and_merge_range_set(out)' to restore range-set invariants
By "bottom", I meant
On Sat, Aug 4, 2018 at 6:18 PM Johannes Schindelin via GitGitGadget
wrote:
> The assertion in question really indicates a bug, when triggered, so we
> might just as well use the sanctioned method to report it.
>
> Signed-off-by: Johannes Schindelin
> ---
> diff --git a/line-log.c b/line-log.c
>
On Sat, Aug 4, 2018 at 6:18 PM Johannes Schindelin via GitGitGadget
wrote:
> Now, I am fairly certain that the changes are correct, but given my track
> record with off-by-one bugs (and once even an off-by-two bug), I would
> really appreciate some thorough review of this code, in particular the
> On 2018-08-05 00:18, Johannes Schindelin via GitGitGadget wrote:
> > Technically, it is okay to have line ranges that touch (i.e. the end of
> > the first range ends just before the next range begins). However, it is
> > inefficient, and when the user provides such touching ranges via
> >
On Sat, Aug 4, 2018 at 6:18 PM Johannes Schindelin via GitGitGadget
wrote:
> When traversing commits and adjusting the ranges, things can get really
> tricky. For example, when the line range of interest encloses several
> hunks of a commit, the line range can actually shrink.
>
> Currently,
On Sun, Aug 5, 2018 at 12:20 AM Jonathan Nieder wrote:
> William Chargin wrote:
> > test_dir_is_empty () {
> > test_path_is_dir "$1" &&
> > - if test -n "$(ls -a1 "$1" | egrep -v '^\.\.?$')"
> > + if test "$(ls -A1 "$1" | wc -c)" != 0
>
> Another portability gotcha: wc output
On Sat, Aug 4, 2018 at 11:33 PM Eric Sunshine wrote:
> On Sat, Aug 4, 2018 at 11:17 PM Jonathan Nieder wrote:
> > So it looks like FreeBSD has modernized and we need to make that
> > conditional in config.mak.uname on $(uname_R). Do you know which
> > version of FreeBSD
On Sat, Aug 4, 2018 at 11:17 PM Jonathan Nieder wrote:
> > utf8.c:486:28: warning: passing 'iconv_ibp *' (aka 'const char **') to
> > parameter
> > of type 'char **' discards qualifiers in nested pointer types
> > [-Wincompatible-pointer-types-discards-qualifiers]
>
> Oh, good catch!
On Sat, Aug 4, 2018 at 2:38 AM Duy Nguyen wrote:
> On Sat, Aug 4, 2018 at 8:11 AM Jonathan Nieder wrote:
> > My main concern is not about them but about other
> > people building from source in order to run (instead of to develop)
> > Git, and by extension, the people they go to for help when it
On Fri, Aug 3, 2018 at 7:14 PM Elijah Newren wrote:
> A test making use of test_must_fail was failing like this:
> fatal: ambiguous argument '|': unknown revision or path not in the working
> tree.
> when the intent was to verify that a specific string was not found
> in the output of the git
On Fri, Aug 3, 2018 at 7:40 PM Eric Sunshine wrote:
> git diff --raw >out &&
> ! grep "" out &&
> (where is a literal TAB)
>
> Since this script has a number of instances of Git commands upstream
> pipes, it may not make sense to fix ju
On Fri, Aug 3, 2018 at 5:47 PM Junio C Hamano wrote:
> Eric Sunshine writes:
> > There doesn't seem to a usage() function defined anywhere (and
> > OPTIONS_SPEC doesn't seem to be used).
>
> Isn't this using the parse-options thing git-sh-setup gives us for
> free?
Yes.
On Fri, Aug 3, 2018 at 5:38 PM Jeff King wrote:
> On Fri, Aug 03, 2018 at 05:33:17PM -0400, Eric Sunshine wrote:
> I suppose so. Frankly I only added that line to appease git-sh-options
> anyway.
> > Should "j" and "f" be "-j" and "-f", res
On Fri, Aug 3, 2018 at 4:52 PM Jeff King wrote:
> [...]
> Let's provide a script that builds and installs the manpages
> for two commits, renders the results using "man", and diffs
> the result. Since this is time-consuming, we'll also do our
> best to avoid repeated work, keeping intermediate
On Fri, Aug 3, 2018 at 5:33 AM Phillip Wood wrote:
> If there isn't some backward compatibility then if git gets upgraded
> while rebase is stopped then the author data will be silently corrupted
> if it contains "'". read_author_ident() will error out but that is only
> used for the root commit.
On Thu, Aug 2, 2018 at 1:27 PM Junio C Hamano wrote:
> Phillip Wood writes:
> > For other interactive rebases this only affects external scripts that
> > read the author script and users whose git is upgraded from the shell
> > version of rebase -i while rebase was stopped when the author
On Thu, Aug 2, 2018 at 7:20 AM Phillip Wood wrote:
> The calling code did not treat NULL as an error. Instead NULL caused
> it to fallback to using the default author when creating the new
> commit. This changed the date and potentially the author of the
> commit which corrupted the author data
On Fri, Aug 3, 2018 at 2:26 AM Jonathan Nieder wrote:
> Eric Sunshine wrote:
> > + if (fd < 0 || fd >= ARRAY_SIZE(want_auto))
> > + BUG("file descriptor out of range: %d", fd);
>
> The indentation looks wrong here.
Yep, that's weird. I can't figur
On Thu, Aug 2, 2018 at 8:13 AM Han-Wen Nienhuys wrote:
> [PATCH 2/2] sideband: highlight keywords in remote sideband output
The -v option of git-format-patch makes it easy to let reviewers know
that this is a new version of a previously-submitted patch series.
This (I think) is the second
On Thu, Aug 2, 2018 at 2:02 PM Jeff King wrote:
> On Thu, Aug 02, 2018 at 01:55:22PM +0200, SZEDER Gábor wrote:
> > This approach uses $(eval), which we haven't used in any of our
> > Makefiles yet. I wonder whether it's portable enough. And it's
> > ugly and complicated.
>
> I
On Thu, Aug 2, 2018 at 1:37 PM Junio C Hamano wrote:
> Johannes Schindelin writes:
> > ACK!
>
> Did you write a buggy caller that would have been caught or helped
> with this change? You did not write the callee that is made more
> defensive with this patch, so I am being curious as to where
On Thu, Aug 2, 2018 at 7:46 AM Han-Wen Nienhuys wrote:
> Sure. My doubt is that it's hard to tell what the state of my patch is
> at any given time.
Understandable. Junio sends out a periodic "What's cooking" email
summarizing the state of patches sent to the list. The most recent one
is here
On Thu, Aug 2, 2018 at 8:13 AM Han-Wen Nienhuys wrote:
> diff --git a/config.h b/config.h
> @@ -178,10 +178,16 @@ struct config_set {
> extern void git_configset_init(struct config_set *cs);
> -extern int git_configset_add_file(struct config_set *cs, const char
> *filename);
> -extern int
On Wed, Aug 1, 2018 at 2:17 PM Junio C Hamano wrote:
> Han-Wen Nienhuys writes:
> > Sorry for being dense, but do you want me to send an updated patch or
> > not based on your and Eric's comments or not?
>
> It would help to see the comments responded with either "such a
> change is not needed
outside the array bounds.
Signed-off-by: Eric Sunshine
---
Just something I noticed while studying this code in relation to a patch
review.
color.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/color.c b/color.c
index b1c24c69de..b0be9ce505 100644
--- a/color.c
+++ b/color.c
@@ -343,6
On Wed, Aug 1, 2018 at 7:25 PM brian m. carlson
wrote:
> On Tue, Jul 31, 2018 at 03:33:27AM -0400, Eric Sunshine wrote:
> > This is a re-roll of [1] which fixes sequencer bugs resulting in commit
> > object corruption when "rebase -i --root" swaps in a new commit as root.
On Wed, Aug 1, 2018 at 3:34 PM Stefan Beller wrote:
> The first patch stands as is unchanged, and the second and third patch
> are different enough that range-diff doesn't want to show a diff.
For future reference, range-diff's --creation-factor tweak may help
here. Depending upon just how
On Wed, Aug 1, 2018 at 11:50 AM Phillip Wood wrote:
> On 31/07/18 22:39, Eric Sunshine wrote:
> > On Tue, Jul 31, 2018 at 7:15 AM Phillip Wood
> > wrote:
> >> + /*
> >> +* write_author_script() used to fail to terminate the
> >
On Tue, Jul 31, 2018 at 1:37 PM Han-Wen Nienhuys wrote:
> diff --git a/config.h b/config.h
> @@ -178,10 +178,16 @@ struct config_set {
> +/*
> + * The int return values in the functions is 1 if not found, 0 if found,
> leaving
> + * the found value in teh 'dest' pointer.
> + */
"teh"?
Instead
On Tue, Jul 31, 2018 at 9:30 PM Hilco Wijbenga wrote:
> On Tue, Jul 31, 2018 at 12:33 AM, Eric Sunshine
> wrote:
> > This is a re-roll of [1] which fixes sequencer bugs resulting in commit
> > object corruption when "rebase -i --root" swaps in a new commit as root.
&
On Tue, Jul 31, 2018 at 7:15 AM Phillip Wood wrote:
> Single quotes should be escaped as \' not \\'. Note that this only
> affects authors that contain a single quote and then only external
> scripts that read the author script and users whose git is upgraded from
> the shell version of rebase -i
On Tue, Jul 31, 2018 at 7:15 AM Phillip Wood wrote:
> The calling code treated NULL as a valid return value, so fix this by
> returning and integer and passing in a parameter to receive the author.
It might be difficult for future readers (those who didn't follow the
discussion) to understand
On Tue, Jul 31, 2018 at 1:37 PM Han-Wen Nienhuys wrote:
> Highlight keywords in remote sideband output.
Prefix with the module you're touching, don't capitalize, and drop the
period. Perhaps:
sideband: highlight keywords in remote sideband output
> The highlighting is done on the
On Tue, Jul 31, 2018 at 8:50 AM Jeff King wrote:
> On Mon, Jul 30, 2018 at 05:38:06PM -0400, Eric Sunshine wrote:
> > I considered that, but it doesn't handle nested here-docs, which we
> > actually have in the test suite. For instance, from t9300-fast-import:
> > [.
On Tue, Jul 31, 2018 at 6:46 AM Eric Sunshine wrote:
> Anyhow, thanks for reading over the series. I appreciate it even if
> our "sense of priority" doesn't always align (as evidenced by your
> review comments and my responses).
To be clear, the changes you suggest all ma
On Tue, Jul 31, 2018 at 6:06 AM Phillip Wood wrote:
> On 31/07/18 08:33, Eric Sunshine wrote:
> > Patch 2/4 of this series conflicts with Akinori MUSHA's
> > 'am/sequencer-author-script-fix' which takes a stab at fixing one of the
> > four (or so) bugs fixed by this s
On Tue, Jul 31, 2018 at 6:02 AM Phillip Wood wrote:
> On 31/07/18 08:33, Eric Sunshine wrote:
> > + /* validate date since fmt_ident() will die() on bad value */
> > + if (parse_date(val[2], )){
> > + warning(_("invalid date format '%s' in '%s'")
On Tue, Jul 31, 2018 at 6:01 AM Phillip Wood wrote:
> Now that the author is correct, can we test_cmp() it against its
> expected value to make sure there are no hidden surprises in the name
> and email in the future. (It would be reassuring to test an author with
> "'" in the name as well but
On Tue, Jul 31, 2018 at 5:50 AM Phillip Wood wrote:
> On 31/07/18 08:33, Eric Sunshine wrote:
> > - sq_dequote(in);
> > + if (!sq_dequote(in)) {
> > + warning(_("bad quoting on %s value in '%s'"),
> >
ot;gentle" cousin, then the manual
timestamp check added here can be retired.
Signed-off-by: Eric Sunshine
---
sequencer.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index 15a66a334c..9b6cdb6ff8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -74
being parsed. Instead, fmt_ident() is
invoked to compose the header after parsing is complete.
Second, fmt_ident() is careful to prevent "crud" from polluting the
composed ident. As with validating GIT_AUTHOR_DATE, this "crud"
avoidance prevents other (possibly hand-ed
he "@" preceding the timestamp is a separate bug which
will be fixed subsequently.)
Signed-off-by: Eric Sunshine
---
sequencer.c | 7 ++-
t/t3404-rebase-interactive.sh | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 78864d9
0180730092929.71114-1-sunsh...@sunshineco.com/
[2]:
https://public-inbox.org/git/1f172fc1-4b51-cdf7-e841-5ca41e209...@talktalk.net/
Eric Sunshine (4):
sequencer: fix "rebase -i --root" corrupting author header
sequencer: fix "rebase -i --root" corrupting author header timezone
-script". Despite
neglecting to NUL-terminate the constructed "author" header, memory is
never accessed (either by read_author_ident() or its caller) beyond the
allocated buffer since a NUL-terminator is present at the end of the
loaded "rebase-merge/author-script" content, and a
each which were
incorrectly indented with six spaces. Also, a purely cosmetic fix.
Signed-off-by: Eric Sunshine
---
Just something I noticed while examining t/chainlint.sed after reading
Jonathan's report[1] of a false-positive.
This is atop 'es/chain-lint-in-subshell' in 'next'.
[1]:
https
On Mon, Jul 30, 2018 at 5:26 PM Thomas Gummerer wrote:
> On 07/30, Johannes Schindelin wrote:
> > On Sun, 29 Jul 2018, Thomas Gummerer wrote:
> > > There's one more thing that I noticed here:
> > >
> > > git range-diff --no-patches
> > > fatal: single arg format requires a symmetric range
On Mon, Jul 30, 2018 at 4:59 PM Jonathan Nieder wrote:
> Eric Sunshine wrote:
> > The subshell linter would normally fold out the here-doc content, but
> > 'sed' isn't a proper programming language, so the linter can't
> > recognize arbitrary here-doc tags. Instead it has
201 - 300 of 3596 matches
Mail list logo
