Re: [PATCH v2] commit: support commit.verbose and --no-verbose
On Sun, May 25, 2014 at 01:24:27AM -0500, Caleb Thompson wrote: ... > would be committed at the bottom of the commit message > template. Note that this diff output doesn't have its > - lines prefixed with '#'. > + lines prefixed with '#'. The `commit.verbose` configuration > + variable can be set to true to implicitly send this option. > + > +--no-verbose:: > + Do not show the unified diff at the bottom of the commit message > + template. This is the default behavior, but can be used to override > + the`commit.verbose` configuration variable. > Why is there two spaces between "diff at"? Needs a space between "the`comm" -> "the `comm". > +cat >check-for-no-diff < +#!$SHELL_PATH > +exec grep -v '^diff --git' "\$1" > +EOF > +chmod +x check-for-no-diff > + Me personally, I would leave it like that for now, since that is the style being used nearby. We'll see what others have to say. I certainly wouldn't convert all the other cases to use test_expect_success. Leave that for another patch. > > @@ -48,6 +54,21 @@ test_expect_success 'verbose diff is stripped out > (mnemonicprefix)' ' > check_message message > ' > > +test_expect_success 'commit shows verbose diff with set commit.verbose' ' > + echo morecontent >file && > + git add file && > + test_config commit.verbose true && > + check_message message > +' > + > +test_expect_success 'commit does not show verbose diff with --no-verbose' ' > + echo morecontent >file && > + git add file && > + test_config commit.verbose true && > + test_set_editor "$PWD/check-for-no-diff" && > + git commit --amend --no-verbose > +' > + I like those better with 'test_config' instead of 'git config', good. Keep working on it, it is looking better :-) -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] commit: support commit.verbose and --no-verbose
On Sun, May 25, 2014 at 01:24:27AM -0500, Caleb Thompson wrote: > Incorporated changes from Duy Nguyen and Jeremiah Mahler. > ... > > +test_expect_success 'commit shows verbose diff with set commit.verbose' ' > + echo morecontent >file && > + git add file && > + test_config commit.verbose true && > + check_message message > +' This test case doesn't appear to be checking for the verbose output. No commit is made so it can't check for the presence of a diff. "check_message message" passes as it did in the test above this (not shown). -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] wording fixes in the user manual and glossary
On 25/05/14 15:50, Jeremiah Mahler wrote: > Some minor wording fixes in the user manual and glossary. > > Signed-off-by: Jeremiah Mahler > --- > Documentation/glossary-content.txt | 2 +- > Documentation/user-manual.txt | 8 > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/Documentation/glossary-content.txt > b/Documentation/glossary-content.txt > index be0858c..4e0b971 100644 > --- a/Documentation/glossary-content.txt > +++ b/Documentation/glossary-content.txt > @@ -1,7 +1,7 @@ > [[def_alternate_object_database]]alternate object database:: > Via the alternates mechanism, a <> > can inherit part of its <> > - from another object database, which is called "alternate". > + from another object database, which is called an "alternate". > > [[def_bare_repository]]bare repository:: > A bare repository is normally an appropriately > diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt > index d33f884..efb3c97 100644 > --- a/Documentation/user-manual.txt > +++ b/Documentation/user-manual.txt > @@ -416,7 +416,7 @@ REVISIONS" section of linkgit:gitrevisions[7]. > Updating a repository with git fetch > > > -Eventually the developer cloned from will do additional work in her > +Eventually the developer will do additional work in her cloned > repository, creating new commits and advancing the branches to point > at the new commits. I agree that the original wording isn't clear but I'm not sure the new wording is any clearer. The paragraph is trying to explain how to fetch upstream changes when they happen. My initial thought was to say "Eventually the developer will do additional work in the upstream repository" but perhaps it is to early to start throwing around terms like upstream. Perhaps just saying "her repository" would be clearest. > > @@ -1811,8 +1811,8 @@ manner. > You can then import these into your mail client and send them by > hand. However, if you have a lot to send at once, you may prefer to > use the linkgit:git-send-email[1] script to automate the process. > -Consult the mailing list for your project first to determine how they > -prefer such patches be handled. > +Consult the mailing list for your project first to determine > +their requirements for submitting patches. > > [[importing-patches]] > Importing patches to a project > @@ -2255,7 +2255,7 @@ $ git checkout test && git merge speed-up-spinlocks > It is unlikely that you would have any conflicts here ... but you might if > you > spent a while on this step and had also pulled new versions from upstream. > > -Some time later when enough time has passed and testing done, you can pull > the > +Sometime later when enough time has passed and testing done, you can pull the > same branch into the `release` tree ready to go upstream. This is where you > see the value of keeping each patch (or patch series) in its own branch. It > means that the patches can be moved into the `release` tree in any order. > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] wording fixes in the user manual and glossary
On Sun, May 25, 2014 at 07:56:41PM +1200, Chris Packham wrote: > On 25/05/14 15:50, Jeremiah Mahler wrote: > > Some minor wording fixes in the user manual and glossary. ... > > > > -Eventually the developer cloned from will do additional work in her > > +Eventually the developer will do additional work in her cloned > > repository, creating new commits and advancing the branches to point > > at the new commits. > > I agree that the original wording isn't clear but I'm not sure the new > wording is any clearer. The paragraph is trying to explain how to fetch > upstream changes when they happen. My initial thought was to say > "Eventually the developer will do additional work in the upstream > repository" but perhaps it is to early to start throwing around terms > like upstream. Perhaps just saying "her repository" would be clearest. > The "developer cloned from will do" didn't sound right to me. But it sounds like my interpretation is not what you were trying to convey. I do like the "upstream" term, it helps describe the local/remote aspect. How about this: Eventually the upstream developer will do additional work in their repository. Creating new commits and advancing the branches to point at the new commits. Then in the next paragraph it will discuss how to use `git fetch` to get these remote changes in to the local repository. -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 11/12] Documentation: add documentation for 'git interpret-trailers'
From: Michael Haggerty > On 04/25/2014 11:07 PM, Christian Couder wrote: >> >> I don't think there is a lot of complexity. >> But maybe I need to explain how it works better. >> Feel free to suggest me sentences I could add. > > I am really excited about having better support for trailers in Git, and > I want to thank you for your work. For me the promise of trailers is > > * A way for users to add information to commits for whatever purpose > they want, without having to convince upstream to built support in. Yeah, I agree this is the main purpose of trailers. > * A standard format for that information, so that all tools can agree > how to read/write trailers without being confused by or breaking > trailers that they didn't know about in advance. Yeah, but don't you think this goal can sometimes go against the previous goal? I mean, if some users for their project think that it's better, for example, if they use trailers like "Fix #42" instead of "Fix: 42", because their bug tracking system supports "Fix #42" better, we should let them do what suits them better, even if Git supports them not as well as if they used "Fix: 42". > * A format that is straightforward enough that it can be machine- > readable with minimum ambiguity. Yeah, but again this could go against the main purpose of trailers above. > * Some command-line tools to make it easy for scripts to work with > trailers, and that serve as a reference implementation that other > Git implementations can imitate. Yeah, ok, as long as we keep in mind the main purpose. > For example, I totally expect that > we will soon want a command-line tool for inquiring about the > presence and contents of trailers, for use in scripting. Eventually > we will want to be able to do stuff like > > git trailers --get-all s-o-b origin/master..origin/next > git rev-list --trailer=s-o-b:gits...@pobox.com master > git trailers --pipe --draft \ > --add-first fixes \ > --append '# You can delete the following line:' \ > --append s-o-b:"$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" \ > --unset private > git trailers --pipe --verify --tidy-up Yeah, feel free to help make this kind of things possible :-) > I think it is really important to nail down the format of trailers > tightly enough that everybody who reads/writes a commit message agrees > about exactly what trailers are there. I think we should have a default format for trailers that is clear, but we should not force users to use this format. Because forcing it would go against the main goal of trailers that you listed first above. > For example the specification > might look something like: > > A commit message can optionally end with a block of trailers. > The trailers, if present, must be separated from the rest of the > commit message by one or more blank lines (lines that contain only > whitespace). There must be no blank lines within the list of > trailers. It is allowed to have blank lines after the trailers. > > Each trailer line must match the following Perl regular > expression: > > ^([A-Za-z0-9_-]+)\s*:\s*(.*[^\s])\s*$ > > The string matching the first group is called the key and the string > matching the second is called the value. Keys are considered to be > case-insensitive [or should they be case-sensitive?]. The > interpretation of values is left entirely up to the application. > Values must not be empty. I tried to be clearer in the v12 I just posted, and I think it should be enough to be very clear. We might want to tweak a little the specifications later, so being too strict might be counter productive. And as other tools might already use trailers in a case-sensitive way and yet other tools in a case-insensitive way, I am not sure we would gain anything by specifying if keys or values should be interpreted in a case-sensitive or case-insensitive way. On the contrary we might upset people already using some of these tools for no good reason. > However, in --draft and --cleanup modes, empty values *are* > allowed, as are comments (lines starting with `core.commentchar`) > within the trailer block. In --draft mode such lines are passed > through unchanged, and in --cleanup mode such lines are removed. I am not sure we should use modes. I think options like "--trim-empty", "--allow-comments", "--allow-empty" might be clearer. > I'm not saying this is the exact definition that we want; I'm just > providing an example of the level of precision that I think is needed. Yeah, but I think too much precision can be counter productive. > With regard to the separator character, my concern is not about how to > document the rules for this one tool. It's more about having really > well-defined rules that are consistent between reading and writing. For > me it seems silly to let "git interpret-trailers" output trailers that > it doesn
Re: [PATCH v2] commit: support commit.verbose and --no-verbose
On Sun, May 25, 2014 at 1:24 PM, Caleb Thompson wrote:
> Duy, you were right about `-V`. Do you know of a simple way to add that
> shortened flag? `OPT_BOOL('v', "verbose", ...)` gives me `-v`, `--verbose`,
> and
> `--no-verbose`, but no `-V` as a shortened form of `--no-verbose`.
No, I don't think parse_options() allows something like that. And we
probably don't want -V for --no-verbose unless it's very often used.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] commit: support commit.verbose and --no-verbose
On Sun, May 25, 2014 at 2:24 AM, Caleb Thompson wrote:
> Incorporated changes from Duy Nguyen and Jeremiah Mahler.
As a courtesy to reviewers, it is helpful to provide a pointer to the
previous submission to give context for the new submission. For
instance, like this [1].
[1]:
http://git.661346.n2.nabble.com/commit-support-commit-verbose-and-no-verbose-td7611617.html
> Jeremiah, I didn't make the changes about `<<-EOF` or `test_expect_success`
> because I'm guessing that keeping the local style of the code intact is more
> important than using those. Do you think it makes sense to refactor the rest
> of
> the test file (t/t7507-commit-verbose.sh) to use those? I could also change
> the
> other `git config` calls to use `test_config`.
Generally speaking, it is important to respect local style, however,
it is also appropriate to include one or more cleanup patches before
your primary changes in order to bring the code in line with current
practices. Conversion to test_config could be such a cleanup patch.
> Duy, you were right about `-V`. Do you know of a simple way to add that
> shortened flag? `OPT_BOOL('v', "verbose", ...)` gives me `-v`, `--verbose`,
> and
> `--no-verbose`, but no `-V` as a shortened form of `--no-verbose`.
At this point, after your email commentary but before the actual
patch, you should have a scissor line -->8-- so that "git am" can
extract your patch automatically from the email.
> commit 1a49356b87c9028e68e731f34790c11a3075f736
Drop this line. It has no meaning outside of your local repository.
> Author: Caleb Thompson
> Date: Fri May 23 11:47:44 2014 -0500
Ditto for the date.
> commit: support commit.verbose and --no-verbose
>
> Add a new configuration variable commit.verbose to implicitly pass
> `--verbose` to `git-commit`. Add `--no-verbose` to commit to negate that
> setting.
The commit message would read just as well or better without the backquotes.
> Signed-off-by: Caleb Thompson
> Reviewed-by: Duy Nguyen
> Reviewed-by: Jeremiah Mahler
Considering that the code in this patch has changed since v1, it's
probably not appropriate to add these Reviewed-by: lines.
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 1932e9b..a245928 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1009,6 +1009,11 @@ commit.template::
> "`~/`" is expanded to the value of `$HOME` and "`~user/`" to the
> specified user's home directory.
>
> +commit.verbose::
> + A boolean to enable/disable inclusion of diff information in the
> + commit message template when using an editor to prepare the commit
> + message. Defaults to false.
> +
> credential.helper::
> Specify an external helper to be called when a username or
> password credential is needed; the helper may consult external
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index 0bbc8f5..d7b50e2 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -282,7 +282,13 @@ configuration variable documented in
> linkgit:git-config[1].
> Show unified diff between the HEAD commit and what
> would be committed at the bottom of the commit message
> template. Note that this diff output doesn't have its
> - lines prefixed with '#'.
> + lines prefixed with '#'. The `commit.verbose` configuration
> + variable can be set to true to implicitly send this option.
> +
> +--no-verbose::
> + Do not show the unified diff at the bottom of the commit message
Already mentioned by Jeremiah: s/diff\s+/diff /
> + template. This is the default behavior, but can be used to override
> + the`commit.verbose` configuration variable.
Also already mentioned: s/the/the /
> -q::
> --quiet::
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 9cfef6c..7978d7f 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1417,6 +1417,10 @@ static int git_commit_config(const char *k, const char
> *v, void *cb)
> sign_commit = git_config_bool(k, v) ? "" : NULL;
> return 0;
> }
> + if (!strcmp(k, "commit.verbose")) {
> + verbose = git_config_bool(k, v);
> + return 0;
> + }
>
> status = git_gpg_config(k, v, NULL);
> if (status)
> @@ -1484,7 +1488,7 @@ int cmd_commit(int argc, const char **argv, const char
> *prefix)
> static struct wt_status s;
> static struct option builtin_commit_options[] = {
> OPT__QUIET(&quiet, N_("suppress summary after successful
> commit")),
> - OPT__VERBOSE(&verbose, N_("show diff in commit message
> template")),
> + OPT_BOOL('v', "verbose", &verbose, N_("show diff in commit
> message template")),
>
> OPT_GROUP(N_("Commit message options")),
> OPT_FILENAME('F', "file", &logfile, N_("read
[PATCH] t9138-git-svn-authors-prog.sh fixups
Several fixups of the t9138-git-svn-authors-prog.sh test script to
follow current recommendations in t/README.
- Fixed a Perl script with a full "#!/usr/bin/perl" shebang
to use write_script() and $PERL_PATH as per t/README.
- Placed svn-authors data setup inside a test_expect_success.
- Fixed trailing quotes to use the same indentation throughout.
Signed-off-by: Jeremiah Mahler
---
t/t9138-git-svn-authors-prog.sh | 35 +--
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/t/t9138-git-svn-authors-prog.sh b/t/t9138-git-svn-authors-prog.sh
index 83cc5fc..d54c37a 100755
--- a/t/t9138-git-svn-authors-prog.sh
+++ b/t/t9138-git-svn-authors-prog.sh
@@ -7,40 +7,39 @@ test_description='git svn authors prog tests'
. ./lib-git-svn.sh
-cat > svn-authors-prog <<'EOF'
-#!/usr/bin/perl
-$_ = shift;
-if (s/-sub$//) {
- print "$_ <$_\@sub.example.com>\n";
-}
-else {
- print "$_ <$_\@example.com>\n";
-}
+write_script svn-authors-prog $PERL_PATH <<-\EOF
+ $_ = shift;
+ if (s/-sub$//) {
+ print "$_ <$_\@sub.example.com>\n";
+ } else {
+ print "$_ <$_\@example.com>\n";
+ }
EOF
-chmod +x svn-authors-prog
-cat > svn-authors <<'EOF'
-ff = FFF FFF
-EOF
+test_expect_success 'svn-authors setup' '
+ cat >svn-authors <<-\EOF
+ ff = FFF FFF
+ EOF
+'
test_expect_success 'setup svnrepo' '
for i in aa bb cc-sub dd-sub ee-foo ff
do
svn mkdir -m $i --username $i "$svnrepo"/$i
done
- '
+'
test_expect_success 'import authors with prog and file' '
git svn clone --authors-prog=./svn-authors-prog \
--authors-file=svn-authors "$svnrepo" x
- '
+'
test_expect_success 'imported 6 revisions successfully' '
(
cd x
test "`git rev-list refs/remotes/git-svn | wc -l`" -eq 6
)
- '
+'
test_expect_success 'authors-prog ran correctly' '
(
@@ -56,7 +55,7 @@ test_expect_success 'authors-prog ran correctly' '
git rev-list -1 --pretty=raw refs/remotes/git-svn~5 | \
grep "^author aa "
)
- '
+'
test_expect_success 'authors-file overrode authors-prog' '
(
@@ -64,7 +63,7 @@ test_expect_success 'authors-file overrode authors-prog' '
git rev-list -1 --pretty=raw refs/remotes/git-svn | \
grep "^author FFF FFF "
)
- '
+'
git --git-dir=x/.git config --unset svn.authorsfile
git --git-dir=x/.git config --unset svn.authorsprog
--
2.0.0.rc4.1.g4a28f16.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
K*i*t*c*h*e*n Design Lancashire Reviews
K*i*t*c*h*e*n Design Lancashire. K*i*t*c*h*e*n worked out at about half of what IKEA wanted for their K*i*t*c*h*e*n. -- View this message in context: http://git.661346.n2.nabble.com/K-i-t-c-h-e-n-Design-Lancashire-Reviews-tp7611671.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
