Re: [PATCH/WIP 4/8] am: split out mbox/maildir patches with git-mailsplit
On Wed, May 27, 2015 at 9:33 AM, Paul Tan pyoka...@gmail.com wrote:
git-am.sh supports mbox, stgit and mercurial patches. Re-implement
support for splitting out mbox/maildirs using git-mailsplit, while also
implementing the framework required to support other patch formats in
the future.
Re-implement support for the --patch-format option (since a5a6755
(git-am foreign patch support: introduce patch_format, 2009-05-27)) to
allow the user to choose between the different patch formats.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
@@ -128,13 +190,32 @@ static void am_next(struct am_state *state)
*/
+/**
+ * parse_options() callback that validates and sets opt-value to the
+ * PATCH_FORMAT_* enum value corresponding to `arg`.
+ */
+static int parse_opt_patchformat(const struct option *opt, const char *arg,
int unset)
+{
+ int *opt_value = opt-value;
+
+ if (!strcmp(arg, mbox))
+ *opt_value = PATCH_FORMAT_MBOX;
+ else
+ return -1;
+ return 0;
+}
+
struct am_state state;
+int opt_patch_format;
Should these two variables be static?
static const char * const am_usage[] = {
N_(git am [options] [(mbox|Maildir)...]),
--
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 4/4] config.c: rewrite ENODEV into EISDIR when mmap fails
On Thu, May 28, 2015 at 10:11:55AM -0700, Junio C Hamano wrote:
if (contents == MAP_FAILED) {
+ if (errno == ENODEV S_ISDIR(st.st_mode))
+ errno = EISDIR;
error(unable to mmap '%s': %s,
config_filename, strerror(errno));
ret = CONFIG_INVALID_FILE;
I think this patch places the magic at the right place, but I
would have preferred to see something more like this:
if (contents == MAP_FAILED) {
if (errno == ENODEV S_ISDIR(st.st_mode))
error(unable to mmap a directory '%s',
config_filename);
else
error(unable to mmap '%s': %s,
config_filename, strerror(errno));
ret = CONFIG_INVALID_FILE;
But that is a very minor preference. I am OK with relying on our
knowledge that strerror(EISDIR) would give something that says the
thing is a directory which is not appropriate for the operation, as
nobody after that strerror() refers to 'errno' in this codepath.
I am OK if you want to switch it. Certainly EISDIR produces good output
on my system, but I don't know if that is universal.
We also know S_ISDIR(st.st_mode) _before_ we actually mmap. So I was
tempted to simply check it beforehand, under the assumption that the
mmap cannot possibly work if we have a directory. But by doing it in the
error code path, then we _know_ we are not affecting the outcome, only
the error message. :)
-Peff
--
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/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
On 05/29/2015 02:11 AM, Matthieu Moy wrote: The point of separating them is that the rename implies a relatively long patch (you have 17 occurences of 'refinfo' in the deletion part of your patch), but it is straightforward to review (apply, run git diff --color-words and press space a few times). But it is no longer so simple once you mix it with anything else. Makes sense, thanks for putting it out. -- Regards, Karthik -- 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/WIP v3 1/4] for-each-ref: extract helper functions out of grab_single_ref()
Karthik Nayak karthik@gmail.com writes: This is a preperatory patch for restructuring 'for-each-ref' and evntually moving most of it to 'ref-filter' to provide the s/evntually/eventually/ The patch looks much nicer now. - int cnt; [...] + REALLOC_ARRAY(cb-grab_array, cb-grab_cnt + 1); + cb-grab_array[cb-grab_cnt++] = ref; - cnt = cb-grab_cnt; - REALLOC_ARRAY(cb-grab_array, cnt + 1); - cb-grab_array[cnt++] = ref; - cb-grab_cnt = cnt; This hunk is not advertized in the commit message, and I had to fight a bit to understand what it does with we're extracting helper functions in mind. It would have been much easier to review in a separate patch entitled for-each-ref: simplify code for example. But I agree that your version is simpler indeed. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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/WIP v3 1/4] for-each-ref: extract helper functions out of grab_single_ref()
On 05/29/2015 01:43 AM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: This is a preperatory patch for restructuring 'for-each-ref' and evntually moving most of it to 'ref-filter' to provide the s/evntually/eventually/ Noted. The patch looks much nicer now. - int cnt; [...] + REALLOC_ARRAY(cb-grab_array, cb-grab_cnt + 1); + cb-grab_array[cb-grab_cnt++] = ref; - cnt = cb-grab_cnt; - REALLOC_ARRAY(cb-grab_array, cnt + 1); - cb-grab_array[cnt++] = ref; - cb-grab_cnt = cnt; This hunk is not advertized in the commit message, and I had to fight a bit to understand what it does with we're extracting helper functions in mind. It would have been much easier to review in a separate patch entitled for-each-ref: simplify code for example. But I agree that your version is simpler indeed. Will put it in a separate patch. Thanks for the review. -- Regards, Karthik -- 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/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
Matthieu Moy matthieu@grenoble-inp.fr writes: Not very important, but two spaces after a period is what one is supposed to do in English. Not everybody follow the rule, but it seems backward to change the code to break it. I am old fashioned enough to personally agree, but this practice that originates in the typewriter era is frowned upon in many circles, including those who work with professional typesetters, I think. Sent off-list to reduce noise level; this is an issue that attracts a lot of useless comments. -- 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
[PATCH] bisect: stop printing raw diff of first bad commit
Signed-off-by: Trevor Saunders tbsau...@tbsaunde.org --- The test change only kind of tests the change in behavior and doesn't seem all that useful. However I'm not sure if its preferable to not even try and test that something isn't output. bisect.c| 7 ++- t/t6030-bisect-porcelain.sh | 3 ++- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/bisect.c b/bisect.c index 10f5e57..244f9e5 100644 --- a/bisect.c +++ b/bisect.c @@ -875,16 +875,13 @@ static void show_diff_tree(const char *prefix, struct commit *commit) init_revisions(opt, prefix); git_config(git_diff_basic_config, NULL); /* no diff UI options */ opt.abbrev = 0; - opt.diff = 1; + opt.diff = 0; /* This is what --pretty does */ opt.verbose_header = 1; opt.use_terminator = 0; opt.commit_format = CMIT_FMT_DEFAULT; - - /* diff-tree init */ - if (!opt.diffopt.output_format) - opt.diffopt.output_format = DIFF_FORMAT_RAW; + opt.always_show_header = 1; log_tree_commit(opt, commit); } diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 06b4868..eb820b2 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -591,7 +591,8 @@ test_expect_success 'test bisection on bare repo - --no-checkout defaulted' ' test \$(git rev-list BISECT_HEAD ^$HASH2 --max-count=1 | wc -l) = 0 \ ../defaulted.log ) - grep $HASH3 is the first bad commit defaulted.log + grep $HASH3 is the first bad commit defaulted.log + test 0 -eq $(grep -c '^:' defaulted.log) ' # -- 2.4.0 -- 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/WIP v3 4/4] ref-filter: move code from 'for-each-ref'
On 05/29/2015 02:05 AM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: Move most of the code from 'for-each-ref' to 'ref-filter' to make it publically available to other commands, this is to unify the code s/publically/publicly/ --- /dev/null +++ b/ref-filter.h Moving file to the .h file should be done in a separate patch. I don't want to review the 1050 lines of cut-and-paste other than by doing the cut-and-paste myself and see if I get the same result, but this part is not as straightforward and needs proper thinking and review. In short: the big code movement should be *only* a cut-and-paste, alone in its own patch. Ok, will separate it into two patches. On overall, we're getting close to an acceptable version for these 4 patches. My advice: prioritize polishing these few patches, so that we can do a detailed review and then consider this part done (either merged in git.git or not, but I'd like to avoid having to review the code movement refactoring again when we move to the next patches). Sure I'll fix changes suggested by you and push to Github probably by tonight (IST), will wait for a day or two to see if there are more suggestions by others on the mailing list before sending a new patch series here. -- Regards, Karthik -- 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/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
Junio C Hamano gits...@pobox.com writes: Sent off-list to reduce noise level; this is an issue that attracts a lot of useless comments. Heh, somehow I screwed up and sent on-list X-. -- 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/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
Karthik Nayak karthik@gmail.com writes:
Rename 'refinfo' to 'ref_array_item' and intoduce 'ref_filter_cbdata'
The fact that you need to use and to describe your changes is a hint
that you can split better.
The patch looks much better, but I think you still need to split more to
make it easier to review.
- * of properties that we need to extract out of objects. refinfo
+ * of properties that we need to extract out of objects. ref_array_item
Not very important, but two spaces after a period is what one is
supposed to do in English. Not everybody follow the rule, but it seems
backward to change the code to break it.
if (flag REF_BAD_NAME) {
- warning(ignoring ref with broken name %s, refname);
- return 0;
+ warning(ignoring ref with broken name %s, refname);
+ return 0;
Whitespace fix mixed with actual code.
-static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct
refinfo *b)
+/* Free all memory allocated for ref_filter_cbdata */
+void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata)
+{
+ int i;
+
+ for (i = 0; i ref_cbdata-array.nr; i++)
+ free(ref_cbdata-array.items[i]);
+ free(ref_cbdata-array.items);
+ ref_cbdata-array.items = NULL;
+ ref_cbdata-array.nr = ref_cbdata-array.alloc = 0;
+}
And this one is a real behavior change, which would be much better
documented in its own patch with a proper commit message (we had a
memory leak before, we didn't care because it happened right before
exiting, but we can't accept that in a clean library).
Reviewing is not just about having a look and seeing if there's
something stupid. Reviewers are actually taking a lot of time to see if
the patch does not introduce new bugs, or looking for better ways to do
the same thing. Be nice with them!
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
On 05/29/2015 01:56 AM, Matthieu Moy wrote:
Karthik Nayak karthik@gmail.com writes:
Rename 'refinfo' to 'ref_array_item' and intoduce 'ref_filter_cbdata'
The fact that you need to use and to describe your changes is a hint
that you can split better.
But the patch alone wouldn't make much sense here, as the whole idea is
the introduction of the new structures and renaming 'refinfo' to
'ref_array_item' was more of a byproduct to go along with the new
structures introduced.
The patch looks much better, but I think you still need to split more to
make it easier to review.
- * of properties that we need to extract out of objects. refinfo
+ * of properties that we need to extract out of objects. ref_array_item
Not very important, but two spaces after a period is what one is
supposed to do in English. Not everybody follow the rule, but it seems
backward to change the code to break it.
I'm just so used to single spacing, Will change it.
if (flag REF_BAD_NAME) {
- warning(ignoring ref with broken name %s, refname);
- return 0;
+ warning(ignoring ref with broken name %s, refname);
+ return 0;
Whitespace fix mixed with actual code.
Noted.
-static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct refinfo
*b)
+/* Free all memory allocated for ref_filter_cbdata */
+void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata)
+{
+ int i;
+
+ for (i = 0; i ref_cbdata-array.nr; i++)
+ free(ref_cbdata-array.items[i]);
+ free(ref_cbdata-array.items);
+ ref_cbdata-array.items = NULL;
+ ref_cbdata-array.nr = ref_cbdata-array.alloc = 0;
+}
And this one is a real behavior change, which would be much better
documented in its own patch with a proper commit message (we had a
memory leak before, we didn't care because it happened right before
exiting, but we can't accept that in a clean library).
Ok will put that into a separate commit.
Reviewing is not just about having a look and seeing if there's
something stupid. Reviewers are actually taking a lot of time to see if
the patch does not introduce new bugs, or looking for better ways to do
the same thing. Be nice with them!
Thanks for the effort from your side, will try to split things as much
as possible and make it easier for Reviewers.
--
Regards,
Karthik
--
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 4/4] config.c: rewrite ENODEV into EISDIR when mmap fails
Jeff King p...@peff.net writes: We also know S_ISDIR(st.st_mode) _before_ we actually mmap. So I was tempted to simply check it beforehand, under the assumption that the mmap cannot possibly work if we have a directory. But by doing it in the error code path, then we _know_ we are not affecting the outcome, only the error message. :) Well, even if your mmap() worked on a directory, having ~/.gitconfig as a directory is wrong in the first place, so... I think what you sent is good as-is, so let's go with them. -- 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: pre-commit hook not run on conflict resolution during rebase
li...@haller-berlin.de (Stefan Haller) writes: When a rebase stops because of a conflict, and I edit the file to resolve the conflict and say git rebase --continue, then the pre-commit hook doesn't run at that point,... From glancing through the githooks manpage, I couldn't see any other hook that would help in this situation. Am I missing something? I do not think so. There may be some fallouts, like negatively affecting folks who have been relying on the current behaviour, but I do not see a fundamental reason why that hook should not trigger there (it may not trigger in the current implementation, but I view it as lack of need so far, not a deliberate omission). I guess the next best solution would be to also have a pre-push hook that performs the same checks again, just in case the bad code managed to get past the pre-commit hook for some reason or other. This feels very redundant, but I guess it would work well. I'd say pre-receive would be the most sensible place to check things like this. Some of your developers may not have pre-commit hook or even run commit --no-verify to bypass the local check, and if you have a central meeting place for the efforts by all your folks, that is where you want to enforce the policy. Checks done anywhere else are what are redundant, including the pre-commit hook in individual developers. You can view them as a way for individual developers to save their time---by choosing to check locally, they make sure their commits do not trigger the pre-receive hook at the central place. -- 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 0/2] config: list only variable names for completion
On Thu, May 28, 2015 at 02:29:33PM +0200, SZEDER Gábor wrote: Fixed misspelled option names in docs and in commit message, incorporated Peff's suggestions, and renamed 'show_only_keys' to 'omit_values' in 1/2. Only minor tweaking in 2/2's commit message. This version looks good to me (though I agree with Junio's proposed squash about the '[value_regex]' documentation). -Peff -- 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/WIP v3 4/4] ref-filter: move code from 'for-each-ref'
Karthik Nayak karthik@gmail.com writes: Move most of the code from 'for-each-ref' to 'ref-filter' to make it publically available to other commands, this is to unify the code s/publically/publicly/ --- /dev/null +++ b/ref-filter.h Moving file to the .h file should be done in a separate patch. I don't want to review the 1050 lines of cut-and-paste other than by doing the cut-and-paste myself and see if I get the same result, but this part is not as straightforward and needs proper thinking and review. In short: the big code movement should be *only* a cut-and-paste, alone in its own patch. On overall, we're getting close to an acceptable version for these 4 patches. My advice: prioritize polishing these few patches, so that we can do a detailed review and then consider this part done (either merged in git.git or not, but I'd like to avoid having to review the code movement refactoring again when we move to the next patches). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
Karthik Nayak karthik@gmail.com writes: On 05/29/2015 01:56 AM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: Rename 'refinfo' to 'ref_array_item' and intoduce 'ref_filter_cbdata' The fact that you need to use and to describe your changes is a hint that you can split better. But the patch alone wouldn't make much sense here, as the whole idea is the introduction of the new structures and renaming 'refinfo' to ref_array_item' was more of a byproduct to go along with the new structures introduced. The point of separating them is that the rename implies a relatively long patch (you have 17 occurences of 'refinfo' in the deletion part of your patch), but it is straightforward to review (apply, run git diff --color-words and press space a few times). But it is no longer so simple once you mix it with anything else. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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] bisect: stop printing raw diff of first bad commit
Trevor Saunders tbsau...@tbsaunde.org writes: Signed-off-by: Trevor Saunders tbsau...@tbsaunde.org --- The test change only kind of tests the change in behavior and doesn't seem all that useful. However I'm not sure if its preferable to not even try and test that something isn't output. As the only objective of this patch is to stop showing that raw format diff output, I think it is sensible to make sure that the output no longer happens. I have a feeling that this patch has some backstory? It may be necessary to summarize it in the log message to explain why this is a good thing to do. diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 06b4868..eb820b2 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -591,7 +591,8 @@ test_expect_success 'test bisection on bare repo - --no-checkout defaulted' ' test \$(git rev-list BISECT_HEAD ^$HASH2 --max-count=1 | wc -l) = 0 \ ../defaulted.log ) - grep $HASH3 is the first bad commit defaulted.log + grep $HASH3 is the first bad commit defaulted.log + test 0 -eq $(grep -c '^:' defaulted.log) ' Your single quotes around the pattern are not doing what you think they are doing. Why not write this line like this instead? ! grep ^: default.log Thanks. -- 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
[PATCH/WIP v3 1/4] for-each-ref: re-structure code for moving to 'ref-filter'
Hello, After the second version of the WIP/PATCH ($gmane/269836), This is a follow up. Changes: * Broke down the first commit into 3 separate commits, cause they were logically different and needed to built up incrementally. * renamed some functions which were made public so that they don't seem vague. * other small changes. -- Regards, Karthik -- 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 1/2] t750*: make tests for commit messages more pedantic
Eric Sunshine sunsh...@sunshineco.com writes: On Tue, May 26, 2015 at 2:15 AM, Patryk Obara patryk.ob...@gmail.com wrote: Currently messages are compared with --pretty=format:%s%b which does not retain raw format of commit message. In result it's not clear what part of expected commit msg is subject and what part is body. Also, it's impossible to test if messages with multiple lines are handled correctly, which may be significant when using nondefault --cleanup. Makes sense. ... +test_expect_success 'template without newline before eof should work with --status' ' It's not clear what should work means. I suppose you mean that the end result should have exactly one newline after the template. Perhaps the test title could indicate the intent more clearly. I agree that what should work in this title is unclear. Because there is nothing wrong in the current system, if a follow-up patch plans to change the established behaviour, the tests in this currently we do not test blank lines, so add tests for them patch should limit themselves to document the current behaviour. Then a follow-up patch that modifies the behaviour can show how the updated behaviour is different and illustrate in what way it is better than the current behaviour. That would be one way to justify the change. -- 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
pre-commit hook not run on conflict resolution during rebase
When a rebase stops because of a conflict, and I edit the file to resolve the conflict and say git rebase --continue, then the pre-commit hook doesn't run at that point, which means that I can commit bad stuff which the pre-commit hook would normally not allow in. We were bitten by this a few times already. (In our case, the hook rejects code that hasn't been run through clang-format. That's easy to forget when resolving conflicts during a rebase.) From glancing through the githooks manpage, I couldn't see any other hook that would help in this situation. Am I missing something? I guess the next best solution would be to also have a pre-push hook that performs the same checks again, just in case the bad code managed to get past the pre-commit hook for some reason or other. This feels very redundant, but I guess it would work well. Any other suggestions? -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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] glossary: add remote, submodule, superproject
On Thu, May 28, 2015 at 9:45 AM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: Noticed-by: Philip Oakley philipoak...@iee.org Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Stefan Beller sbel...@google.com --- Documentation/glossary-content.txt | 17 + 1 file changed, 17 insertions(+) The updates in this version relative to the previous one looks very good, at least to me. A bit more comments. diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index bf383c2..23ab692 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -469,6 +469,11 @@ The most notable example is `HEAD`. def_push,push to describe the mapping between remote def_ref,ref and local ref. +[[def_remote]]remote repository:: + A def_repository,repository which is used to track the same + project but resides somewhere else. To communicate with remotes, + see def_fetch,fetch or def_push,push. + The last sentence sounds a tiny bit strange, in that I have to do a bit more than just see the explanation of these commands in order to communicate with remotes. Maybe s/see/use/ here? But it probably is just me. @@ -515,6 +520,18 @@ The most notable example is `HEAD`. is created by giving the `--depth` option to linkgit:git-clone[1], and its history can be later deepened with linkgit:git-fetch[1]. +[[def_submodule]]submodule:: + A def_repository,repository that holds the history of a + separate project inside another repository (the latter of + which is called def_superproject, superproject). The + containing superproject knows about the names of (but does + not hold copies of) commit objects of the contained submodules. I agree with one point you mentioned in one of your messages, which is that a submodule is not aware that it is used as part of a larger project. That makes me wonder if the last sentence sits better in the description of the superproject, rather than the description of the submodule. Moved in the upcoming reroll. +[[def_superproject]]superproject:: + A def_repository,repository that references other repositories + inside itself as def_submodule,submodules. Perhaps repositories of other projects? Does inside make it clear enough that we are talking about the relationship between working trees of the superproject and submodules? A def_repository,repository that references repositories of other projects in its working tree as def_submodule,submodules. + The superproject + tracks only the remote and the name of the submodule. I am not sure what this sentence means [*1*], and I do not know if (a corrected version of) such a description is necessary here. When looking at submodules and subtrees I feel they behave similar to symbolic and hard links. If you delete the remote of the submodule you need to take care when dealing with the superproject, similar to repairing a dangling symlink. (Is it gone or just moved? Where do I point it now?) My intend here was to show that submodules are fragile like symlinks are. Usually a repository (or a file in that analogy) is quite self contained, if you have a copy of the repository, you can do lots of operation on it like reading, changing(writing), moving. If there is a broken (git/sym-)link reading in full becomes a hassle, as parts are missing. I am not sure if the discussion belongs into the glossary though. But where would you start looking for information if you want to decided whether to use submodules or subtrees? Thanks. [Footnote] *1* The superproject records a bit more than remote and name in .gitmodules, and of course it records the history of the paths that the submodule is bound to over time, with specific commits from the submodule in its history. -- 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: [RFC/PATCH v2] create a skeleton for the command git rebase --status
Matthieu Moy matthieu@grenoble-inp.fr writes: But I think there are more relevant information to show (e.g. list of already applied commits, remaining list of commits, possibly truncated if the list is overly long, and information that rebase gave you when stopping like the path to the file being applied). Having them all in git status would make the output really long, for little benefit in day-to-day use. Sorry, I do not quite agree with this reasoning. Isn't git status during a rebase that shows really long information to help the rebase operation a good thing? In day-to-day use when you are not in the middle of rebase, the command would not show what remains to be done, would it? I may be biased, because I rarely use 'git status' while running 'git rebase' (with or without interactive). But to me, 'git diff' would be a more appropriate tool to help me unstuck in managing the current step of conflict resolution than 'git status' gives me during either a rebase or a merge as Unmerged paths anyway. If this topic enhances 'git status' with the in-progress rebase information, I'd view it as turning 'git status' from 'a more or less useless command during rebase' to 'a useful command'. -- 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 1/3] t4150-am: refactor and clean common setup
On Tue, May 26, 2015 at 5:32 PM, Remi Lespinet
remi.lespi...@ensimag.grenoble-inp.fr wrote:
Add new functions to keep the setup cleaner:
- setup_temporary_branch: creates a new branch, check it out
and automatically delete it after the test is over
- setup_fixed_branch: creates a fixed branch, which can be re-used
in later tests
See below for review comments beyond those already provided by Paul...
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 306e6f3..8370951 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -4,6 +4,20 @@ test_description='git am running'
. ./test-lib.sh
+setup_temporary_branch () {
+ tmp_name=${2-temporary}
+ git reset --hard
+ rm -fr .git/rebase-apply
+ test_when_finished git checkout $1 git branch -D $tmp_name
+ git checkout -b $tmp_name $1
+}
+
+setup_fixed_branch () {
+ git reset --hard
+ rm -fr .git/rebase-apply
+ git checkout -b $1 $2
+}
+
test_expect_success 'setup: messages' '
cat msg -\EOF
second
@@ -143,9 +157,7 @@ test_expect_success setup '
'
test_expect_success 'am applies patch correctly' '
- rm -fr .git/rebase-apply
- git reset --hard
- git checkout first
+ setup_temporary_branch first
This is confusing. The commit message seems to advertise this patch as
primarily a refactoring change (pulling boilerplate out of tests and
into a new function), but the operation of that new function is
surprisingly different from the boilerplate it's replacing: The old
code did not create a branch, the new function does.
If your intention really is to create new branches in tests which
previously did not need throwaway branches, then the commit message
should state that as its primary purpose and explain why doing so is
desirable, since it is not clear from this patch what you gain from
doing so.
Moreover, typically, you should only either refactor or change
behavior in a single patch, not both. For instance, patch 1 would add
the new function and update tests to call it in place of the
boilerplate; and patch 2 would change behavior (such as creating a
temporary branch).
On the other hand, if these tests really don't benefit from having a
throw-away branch, then this change seems suspect and obscures the
intent of the test rather than clarifying or simplifying.
test_tick
git am patch1
test_path_is_missing .git/rebase-apply
@@ -275,9 +273,7 @@ test_expect_success 'am --keep-non-patch really keeps the
non-patch part' '
'
test_expect_success 'am -3 falls back to 3-way merge' '
- rm -fr .git/rebase-apply
- git reset --hard
- git checkout -b lorem2 master2
+ setup_fixed_branch lorem2 master2
sed -n -e 3,\$p msg file
head -n 9 msg file
git add file
@@ -289,9 +285,7 @@ test_expect_success 'am -3 falls back to 3-way merge' '
'
test_expect_success 'am -3 -p0 can read --no-prefix patch' '
- rm -fr .git/rebase-apply
- git reset --hard
- git checkout -b lorem3 master2
+ setup_temporary_branch lorem2
Unlike the test prior to this one which creates a fixed branch (via
setup_fixed_branch) named 'lorem2' which is used by other tests, the
'lorem3' branch in this test is never used elsewhere, so
setup_temporary_branch is used instead. Makes sense.
sed -n -e 3,\$p msg file
head -n 9 msg file
git add file
@@ -303,10 +297,8 @@ test_expect_success 'am -3 -p0 can read --no-prefix
patch' '
'
test_expect_success 'am can rename a file' '
+ setup_temporary_branch lorem
grep ^rename from rename.patch
- rm -fr .git/rebase-apply
- git reset --hard
- git checkout lorem^0
In other cases, you insert the setup_temporary_branch() invocation
where the old code resided. Why the difference in this test (and
others following)?
git am rename.patch
test_path_is_missing .git/rebase-apply
git update-index --refresh
@@ -349,11 +335,9 @@ test_expect_success 'am -3 -q is quiet' '
'
test_expect_success 'am pauses on conflict' '
- rm -fr .git/rebase-apply
- git reset --hard
- git checkout lorem2^^
+ setup_temporary_branch lorem2^^
test_must_fail git am lorem-move.patch
- test -d .git/rebase-apply
+ test_path_is_dir .git/rebase-apply
Sneaking in unrelated change. This sort of cleanup should go in a
patch of its own.
'
test_expect_success 'am --skip works' '
--
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 4/4] config.c: rewrite ENODEV into EISDIR when mmap fails
Jeff King p...@peff.net writes:
If we try to mmap a directory, we'll get ENODEV. This
translates to no such device for the user, which is not
very helpful. Since we've just fstat()'d the file, we can
easily check whether the problem was a directory to give a
better message.
Signed-off-by: Jeff King p...@peff.net
---
It feels a bit wrong to put this magic conversion here, and not in
xmmap. But of course xmmap does not have the stat information.
...
diff --git a/config.c b/config.c
index e7dc155..29fa012 100644
--- a/config.c
+++ b/config.c
@@ -2056,6 +2056,8 @@ int git_config_set_multivar_in_file(const char
*config_filename,
contents = xmmap_gently(NULL, contents_sz, PROT_READ,
MAP_PRIVATE, in_fd, 0);
if (contents == MAP_FAILED) {
+ if (errno == ENODEV S_ISDIR(st.st_mode))
+ errno = EISDIR;
error(unable to mmap '%s': %s,
config_filename, strerror(errno));
ret = CONFIG_INVALID_FILE;
I think this patch places the magic at the right place, but I
would have preferred to see something more like this:
if (contents == MAP_FAILED) {
if (errno == ENODEV S_ISDIR(st.st_mode))
error(unable to mmap a directory '%s',
config_filename);
else
error(unable to mmap '%s': %s,
config_filename, strerror(errno));
ret = CONFIG_INVALID_FILE;
But that is a very minor preference. I am OK with relying on our
knowledge that strerror(EISDIR) would give something that says the
thing is a directory which is not appropriate for the operation, as
nobody after that strerror() refers to 'errno' in this codepath.
Thanks. The patches were a pleasant read.
--
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
[PATCH/WIP v3 4/4] ref-filter: move code from 'for-each-ref'
Move most of the code from 'for-each-ref' to 'ref-filter' to make
it publically available to other commands, this is to unify the code
of 'tag -l', 'branch -l' and 'for-each-ref' so that they can share
their implementations with each other.
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
Makefile |1 +
builtin/for-each-ref.c | 1089 +---
ref-filter.c | 1051 ++
ref-filter.h | 66 +++
4 files changed, 1119 insertions(+), 1088 deletions(-)
create mode 100644 ref-filter.c
create mode 100644 ref-filter.h
diff --git a/Makefile b/Makefile
index 36655d5..068d313 100644
--- a/Makefile
+++ b/Makefile
@@ -761,6 +761,7 @@ LIB_OBJS += reachable.o
LIB_OBJS += read-cache.o
LIB_OBJS += reflog-walk.o
LIB_OBJS += refs.o
+LIB_OBJS += ref-filter.o
LIB_OBJS += remote.o
LIB_OBJS += replace_object.o
LIB_OBJS += rerere.o
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index f045def..2f11c01 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -1,1095 +1,8 @@
#include builtin.h
#include cache.h
#include refs.h
-#include object.h
-#include tag.h
-#include commit.h
-#include tree.h
-#include blob.h
-#include quote.h
#include parse-options.h
-#include remote.h
-#include color.h
-
-/* Quoting styles */
-#define QUOTE_NONE 0
-#define QUOTE_SHELL 1
-#define QUOTE_PERL 2
-#define QUOTE_PYTHON 4
-#define QUOTE_TCL 8
-
-typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
-
-struct atom_value {
- const char *s;
- unsigned long ul; /* used for sorting when not FIELD_STR */
-};
-
-struct ref_sort {
- struct ref_sort *next;
- int atom; /* index into used_atom array */
- unsigned reverse : 1;
-};
-
-struct ref_array_item {
- unsigned char objectname[20];
- int flag;
- const char *symref;
- struct atom_value *value;
- char *refname;
-};
-
-struct ref_array {
- int nr, alloc;
- struct ref_array_item **items;
-};
-
-struct ref_filter {
- const char **name_patterns;
-};
-
-struct ref_filter_cbdata {
- struct ref_array array;
- struct ref_filter filter;
-};
-
-static struct {
- const char *name;
- cmp_type cmp_type;
-} valid_atom[] = {
- { refname },
- { objecttype },
- { objectsize, FIELD_ULONG },
- { objectname },
- { tree },
- { parent },
- { numparent, FIELD_ULONG },
- { object },
- { type },
- { tag },
- { author },
- { authorname },
- { authoremail },
- { authordate, FIELD_TIME },
- { committer },
- { committername },
- { committeremail },
- { committerdate, FIELD_TIME },
- { tagger },
- { taggername },
- { taggeremail },
- { taggerdate, FIELD_TIME },
- { creator },
- { creatordate, FIELD_TIME },
- { subject },
- { body },
- { contents },
- { contents:subject },
- { contents:body },
- { contents:signature },
- { upstream },
- { symref },
- { flag },
- { HEAD },
- { color },
-};
-
-/*
- * An atom is a valid field atom listed above, possibly prefixed with
- * a * to denote deref_tag().
- *
- * We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects. ref_array_item
- * structure will hold an array of values extracted that can be
- * indexed with the atom number, which is an index into this
- * array.
- */
-static const char **used_atom;
-static cmp_type *used_atom_type;
-static int used_atom_cnt, need_tagged, need_symref;
-static int need_color_reset_at_eol;
-
-/*
- * Used to parse format string and sort specifiers
- */
-int parse_ref_filter_atom(const char *atom, const char *ep)
-{
- const char *sp;
- int i, at;
-
- sp = atom;
- if (*sp == '*' sp ep)
- sp++; /* deref */
- if (ep = sp)
- die(malformed field name: %.*s, (int)(ep-atom), atom);
-
- /* Do we have the atom already used elsewhere? */
- for (i = 0; i used_atom_cnt; i++) {
- int len = strlen(used_atom[i]);
- if (len == ep - atom !memcmp(used_atom[i], atom, len))
- return i;
- }
-
- /* Is the atom a valid one? */
- for (i = 0; i ARRAY_SIZE(valid_atom); i++) {
- int len = strlen(valid_atom[i].name);
- /*
-* If the atom name has a colon, strip it and everything after
-* it off - it specifies the format for this entry, and
-* shouldn't be used for checking against the valid_atom
-* table.
-*/
- const char *formatp = strchr(sp, ':');
- if (!formatp ||
[PATCH/WIP v3 2/4] for-each-ref: introduce new structures for better organisation
Rename 'refinfo' to 'ref_array_item' and intoduce 'ref_filter_cbdata'
which will hold 'ref_filter' (Conditions to filter the refs on) and
'ref_array' (The array of ref_array_items). Modify the code to use
these new structures.
Re-order the fields in 'ref_array_item' so that refname can be
eventually converted to a FLEX_ARRAY.
Introduce 'ref_filter_clear_data' to clear all allocated memory.
This is a preparatory patch to eventually move code from 'for-each-ref'
to 'ref-filter' and making it publically available.
Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by:
Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/for-each-ref.c | 103 +
1 file changed, 61 insertions(+), 42 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 919d45e..d9fd512 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -31,12 +31,26 @@ struct ref_sort {
unsigned reverse : 1;
};
-struct refinfo {
- char *refname;
+struct ref_array_item {
unsigned char objectname[20];
int flag;
const char *symref;
struct atom_value *value;
+ char *refname;
+};
+
+struct ref_array {
+ int nr, alloc;
+ struct ref_array_item **items;
+};
+
+struct ref_filter {
+ const char **name_patterns;
+};
+
+struct ref_filter_cbdata {
+ struct ref_array array;
+ struct ref_filter filter;
};
static struct {
@@ -85,7 +99,7 @@ static struct {
* a * to denote deref_tag().
*
* We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects. refinfo
+ * of properties that we need to extract out of objects. ref_array_item
* structure will hold an array of values extracted that can be
* indexed with the atom number, which is an index into this
* array.
@@ -622,7 +636,7 @@ static inline char *copy_advance(char *dst, const char *src)
/*
* Parse the object referred by ref, and grab needed value.
*/
-static void populate_value(struct refinfo *ref)
+static void populate_value(struct ref_array_item *ref)
{
void *buf;
struct object *obj;
@@ -821,7 +835,7 @@ static void populate_value(struct refinfo *ref)
* Given a ref, return the value for the atom. This lazily gets value
* out of the object by calling populate value.
*/
-static void get_value(struct refinfo *ref, int atom, struct atom_value **v)
+static void get_value(struct ref_array_item *ref, int atom, struct atom_value
**v)
{
if (!ref-value) {
populate_value(ref);
@@ -830,12 +844,6 @@ static void get_value(struct refinfo *ref, int atom,
struct atom_value **v)
*v = ref-value[atom];
}
-struct grab_ref_cbdata {
- struct refinfo **grab_array;
- const char **grab_pattern;
- int grab_cnt;
-};
-
/*
* Given a refname, return 1 if the refname matches with one of the patterns
* while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
@@ -860,12 +868,12 @@ static int match_name_as_path(const char **pattern, const
char *refname)
return 0;
}
-/* Allocate space for a new refinfo and copy the objectname and flag to it */
-static struct refinfo *new_refinfo(const char *refname,
- const unsigned char *objectname,
- int flag)
+/* Allocate space for a new ref_array_item and copy the objectname and flag to
it */
+static struct ref_array_item *new_ref_array_item(const char *refname,
+const unsigned char
*objectname,
+int flag)
{
- struct refinfo *ref = xcalloc(1, sizeof(struct refinfo));
+ struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item));
ref-refname = xstrdup(refname);
hashcpy(ref-objectname, objectname);
ref-flag = flag;
@@ -879,26 +887,39 @@ static struct refinfo *new_refinfo(const char *refname,
*/
static int grab_single_ref(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
{
- struct grab_ref_cbdata *cb = cb_data;
- struct refinfo *ref;
+ struct ref_filter_cbdata *ref_cbdata = cb_data;
+ struct ref_filter *filter = ref_cbdata-filter;
+ struct ref_array_item *ref;
if (flag REF_BAD_NAME) {
- warning(ignoring ref with broken name %s, refname);
- return 0;
+ warning(ignoring ref with broken name %s, refname);
+ return 0;
}
- if (*cb-grab_pattern !match_name_as_path(cb-grab_pattern, refname))
+ if (*filter-name_patterns
!match_name_as_path(filter-name_patterns, refname))
return 0;
- ref = new_refinfo(refname, sha1, flag);
+ ref = new_ref_array_item(refname, sha1, flag);
-
[PATCH/WIP v3 3/4] for-each-ref: rename some functions and make them public
Rename some of the functions and make them publically available.
This is a preparatory step for moving code from 'for-each-ref'
to 'ref-filter' to make meaningful, targeted services available to
other commands via public APIs.
Based-on-patch-by: Jeff King p...@peff.net
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/for-each-ref.c | 45 +++--
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index d9fd512..f045def 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -112,7 +112,7 @@ static int need_color_reset_at_eol;
/*
* Used to parse format string and sort specifiers
*/
-static int parse_atom(const char *atom, const char *ep)
+int parse_ref_filter_atom(const char *atom, const char *ep)
{
const char *sp;
int i, at;
@@ -189,7 +189,7 @@ static const char *find_next(const char *cp)
* Make sure the format string is well formed, and parse out
* the used atoms.
*/
-static int verify_format(const char *format)
+int verify_ref_format(const char *format)
{
const char *cp, *sp;
@@ -201,7 +201,7 @@ static int verify_format(const char *format)
if (!ep)
return error(malformed format string %s, sp);
/* sp points at %( and ep points at the closing ) */
- at = parse_atom(sp + 2, ep);
+ at = parse_ref_filter_atom(sp + 2, ep);
cp = ep + 1;
if (skip_prefix(used_atom[at], color:, color))
@@ -408,7 +408,7 @@ static void grab_date(const char *buf, struct atom_value
*v, const char *atomnam
/*
* We got here because atomname ends in date or datesomething;
* it's not possible that something is not :format because
-* parse_atom() wouldn't have allowed it, so we can assume that no
+* parse_ref_filter_atom() wouldn't have allowed it, so we can assume
that no
* : means no format is specified, and use the default.
*/
formatp = strchr(atomname, ':');
@@ -835,7 +835,7 @@ static void populate_value(struct ref_array_item *ref)
* Given a ref, return the value for the atom. This lazily gets value
* out of the object by calling populate value.
*/
-static void get_value(struct ref_array_item *ref, int atom, struct atom_value
**v)
+static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct
atom_value **v)
{
if (!ref-value) {
populate_value(ref);
@@ -882,10 +882,10 @@ static struct ref_array_item *new_ref_array_item(const
char *refname,
}
/*
- * A call-back given to for_each_ref(). Filter refs and keep them for
+ * A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
*/
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
{
struct ref_filter_cbdata *ref_cbdata = cb_data;
struct ref_filter *filter = ref_cbdata-filter;
@@ -925,8 +925,8 @@ static int cmp_ref_sort(struct ref_sort *s, struct
ref_array_item *a, struct ref
int cmp;
cmp_type cmp_type = used_atom_type[s-atom];
- get_value(a, s-atom, va);
- get_value(b, s-atom, vb);
+ get_ref_atom_value(a, s-atom, va);
+ get_ref_atom_value(b, s-atom, vb);
switch (cmp_type) {
case FIELD_STR:
cmp = strcmp(va-s, vb-s);
@@ -958,7 +958,7 @@ static int compare_refs(const void *a_, const void *b_)
return 0;
}
-static void sort_refs(struct ref_sort *sort, struct ref_array *array)
+void sort_ref_array(struct ref_sort *sort, struct ref_array *array)
{
ref_sort = sort;
qsort(array-items, array-nr, sizeof(struct ref_array_item *),
compare_refs);
@@ -1028,7 +1028,7 @@ static void emit(const char *cp, const char *ep)
}
}
-static void show_ref(struct ref_array_item *info, const char *format, int
quote_style)
+void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style)
{
const char *cp, *sp, *ep;
@@ -1038,7 +1038,7 @@ static void show_ref(struct ref_array_item *info, const
char *format, int quote_
ep = strchr(sp, ')');
if (cp sp)
emit(cp, sp);
- get_value(info, parse_atom(sp + 2, ep), atomv);
+ get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep),
atomv);
print_value(atomv, quote_style);
}
if (*cp) {
@@ -1057,18 +1057,19 @@ static void show_ref(struct ref_array_item *info, const
char *format, int quote_
putchar('\n');
}
-static struct ref_sort *default_sort(void)
+/* If no sorting option is given, use
Re: [PATCH/RFC] send-email: allow multiple emails using --cc --to and --bcc
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes: From: Jorge Juan Garcia Garcia jorge-juan.garcia-gar...@ensimag.imag.fr Add the possibility to use a list of emails separated by commas in flags --cc --to and --bcc instead of having to use one flag per email address. The use-case is to copy-paste a list of addresses from an email. This change makes it so that we no longer need to cut the list. s/Add the possibility to use/Accept/; I am not sure having to use is a good characterization. Accept a list of emails separated by commas in flags --cc, --to and --bcc. Multiple addresses can already be given by using these options multiple times, but it is more convenient to allow cutting-and-pasting a list of addresses from the header of an existing e-mail message, which already lists them as comma-separated list, as a value to a single parameter. perhaps? The format of email list handled is basic for now: $ git send-email --to='Foo f...@example.com, b...@example.com' We thought it would be nice to have a first-step version which works Separate displayed material from the body text with blank lines, i.e. The format of email list handled is basic for now: $ git send-email --to='Foo ... We thought it would be nice to have a first-step version which works We thought? before handling more complex ones such as names with commas: $ git send-email --to='Foo, Bar foo...@example.com' Shouldn't this example send to two users, i.e. a local user Foo and the mailbox 'foobar' at example.com whose human-readable name is Bar? If so, that is a bad example to illustrate the aspiration for the finished patch? This artificial limitation is imposed by 79ee555b (Check and document the options to prevent mistakes, 2006-06-21). It is unclear from this sentence what you are doing with the limitation (is it artificial? do we have to call it artificial?). The reader of the log message cannot tell if you are keeping it, or if you are lifting it with this patch. Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr Signed-off-by: Jorge Juan Garcia Garcia jorge-juan.garcia-gar...@ensimag.imag.fr Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr Contributions-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr You touched the patch or sent it out, so you need to give your own sign-off. Thanks. -- 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 1/3] t4150-am: refactor and clean common setup
On Thu, May 28, 2015 at 3:09 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Tue, May 26, 2015 at 5:32 PM, Remi Lespinet
remi.lespi...@ensimag.grenoble-inp.fr wrote:
+setup_temporary_branch () {
+ tmp_name=${2-temporary}
I forgot to mention the broken -chain here. Although the missing
doesn't actively hurt the function today, someone may someday insert
code above the 'tmp_name=' line without noticing the lack of , and
the test won't notice a failure in that newly added code. Thus, it's
better to keep the -chain intact throughout.
+ git reset --hard
+ rm -fr .git/rebase-apply
+ test_when_finished git checkout $1 git branch -D $tmp_name
+ git checkout -b $tmp_name $1
+}
--
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: Bug: .gitconfig folder
Jeff King p...@peff.net writes: On Wed, May 27, 2015 at 03:38:12PM -0700, Junio C Hamano wrote: The patch was meant to be a tongue-in-cheek tangent that is a vast improvement for cases where we absolutely need to use mmap but does not help the OP at all ;-) I do not think there is any need for the config reader to read the existing file via mmap interface; just open it, strbuf_read() the whole thing (and complain when it cannot) and we should be ok. Or do we write back through the mmaped region or something? No, I think we must never do that in our code because our compat mmap implementation uses pread(). So all maps must be MAP_PRIVATE (and our compat mmap barfs if it is not). I started to go the strbuf_read() route, but it just felt so dirty to change the way the code works only to try to get a better error message. Hmm. I actually thought that we long time ago updated the system to read small loose object files via read(2) instead of mmap(2) purely as an optimization, as mmap(2) is a bad match if you are going to read the whole thing from the beginning to the end anyway, and the why not strbuf_read() the whole configuration file was a suggestion along that line. But apparently we do not have such an optimization in read_object() codepath, perhaps I was hallucinating X-. ... but the config-writing code is such a tangled mess that I don't want to spend the time or risk the regressions. That part I agree with. I was kinda hoping that the previous GSoC would clean it up, but that did not happen. -- 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/RFC 1/2] git-rebase -i: Add key word drop to remove a commit
Hi Stefan, On 2015-05-27 23:47, Stefan Beller wrote: On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano gits...@pobox.com wrote: Talking about ideas: I sometimes have the wrong branch checked out when doing a small fixup commit. So I want to drop that patch from the current branch and apply it to another branch. Maybe an instruction like cherry-pick-to-branch-(and-do-not-apply-here) would help me there. Oh, is it wish-anything time? *claps-his-hands* I would wish for a graphical tool which visualizes the commit graph in a visually pleasing manner, where I can select one or more commits and drop them onto a commit in the graph, and then it goes and magically cherry-picks-and-drops them. :-) Ciao, Dscho -- 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 3/3] git-am: add am.threeWay config variable
Paul Tan pyoka...@gmail.com writes: diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt index 0d8ba48..3190c05 100644 --- a/Documentation/git-am.txt +++ b/Documentation/git-am.txt @@ -89,11 +89,13 @@ default. You can use `--no-utf8` to override this. linkgit:git-mailinfo[1]). -3:: ---3way:: +--[no-]3way:: There's no need to mention --no-3way,... Actually, we prefer to do it this way: -3:: --3way:: --no-3way:: Describe what --3way does here. $ git grep -e '^--no-' -e '^--\[no-\]' Documentation/ When the patch does not apply cleanly, fall back on 3-way merge if the patch records the identity of blobs it is supposed to apply to and we have those blobs - available locally. + available locally. `am.threeWay` configuration variable + can be used to specify the default behaviour. `--no-3way` + is useful to override `am.threeWay`. Usually configuration settings are mentioned in a separate section in the documentation CONFIGURATION (or not mentioned at all). I can go either way, actually. But if the description mentions am.threeWay as a way to tweak the default, it also should spell out the default when the configuration is not there at all. Also, there's no need to mention that --no-3way can be used to mention the configuration, as its usual (and expected) that the configuration value sets the default behavior, and the command-line switch can override i. Yes. Also --3way is useful to override `am.threeWay` set to `false` ;-) To end off, some off-tangent issues that are not related to the patch series in question, but since I'm looking at git-am.sh I've noticed that in the block above that initializes all the variables, sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort= messageid= resolvemsg= resume= scissors= no_inbody_headers= git_apply_opt= committer_date_is_author_date= ignore_date= allow_rerere_autoupdate= gpg_sign_opt= threeway is not initialized at all, and thus I think running threeway=t git am blah will affect the behavior of git-am. Correct. I overlooked this when I originally did threeway. Perhaps a preparatory bugfix patch is warranted before this one. Also, I noticed that we do not check for --no-interactive, --no-signoff, --no-keep, --no-whitespace, etc. Even though adding support for them would not hurt, lack of these are OK, as long as we do not have configuration variables to tweak their defaults. -- 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: [RFC/PATCH v2] create a skeleton for the command git rebase --status
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: If this topic enhances 'git status' with the in-progress rebase information, I'd view it as turning 'git status' from 'a more or less useless command during rebase' to 'a useful command'. For day-to-day operations, what we already have in status already qualifies as 'useful command' to me: $ git status rebase in progress; onto 7f9a792 You are currently rebasing branch 'master' on '7f9a792'. (fix conflicts and then run git rebase --continue) (use git rebase --skip to skip this patch) (use git rebase --abort to check out the original branch) Not really. How would you decide if 7f9a792 is worth keeping or rebase is better be aborted without knowing where you are? I like the output of git status to be concise. Sure, as long as concise and useful, I am all for it. The above however does not show anything I already know in my prompt. I would say no thanks to concise and useless. OTOH, there are tons of information in .git/rebase-merge/ that could be displayed to the user. Surely, that is why git status during a rebase should show them. -- 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/RFC 1/2] git-rebase -i: Add key word drop to remove a commit
On Thu, May 28, 2015 at 10:06 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi Stefan, On 2015-05-27 23:47, Stefan Beller wrote: On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano gits...@pobox.com wrote: Talking about ideas: I sometimes have the wrong branch checked out when doing a small fixup commit. So I want to drop that patch from the current branch and apply it to another branch. Maybe an instruction like cherry-pick-to-branch-(and-do-not-apply-here) would help me there. Oh, is it wish-anything time? *claps-his-hands* Maybe my wording was bad, sorry about that. I think throwing around ideas (which are closely related to what is trying to be accomplished here IMHO) is not necessarily bad, but the exchange of ideas helps in understanding the issue better (I like your idea as I have not thought about it that way., What about use case X, Your idea is nuts because Y) I would wish for a graphical tool which visualizes the commit graph in a visually pleasing manner, where I can select one or more commits and drop them onto a commit in the graph, and then it goes and magically cherry-picks-and-drops them. Drag and Drop, I get it. ;) Additionally, if dropped on an unnamed branch, it should come up with a reasonable new branch name. :-) Ciao, Dscho -- 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
[PATCH/WIP v3 1/4] for-each-ref: extract helper functions out of grab_single_ref()
Extract two helper functions out of grab_single_ref(). Firstly,
new_refinfo() which is used to allocate memory for a new refinfo
structure and copy the objectname, refname and flag to it.
Secondly, match_name_as_path() which when given an array of patterns
and the refname checks if the refname matches any of the patterns
given while the pattern is a pathname, also while supporting wild
characters.
This is a preperatory patch for restructuring 'for-each-ref' and
evntually moving most of it to 'ref-filter' to provide the
functionality to similar commands via public API's.
Helped-by: Junio C Hamano gits...@pobox.com
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/for-each-ref.c | 76 --
1 file changed, 43 insertions(+), 33 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 83f9cf9..919d45e 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -837,6 +837,43 @@ struct grab_ref_cbdata {
};
/*
+ * Given a refname, return 1 if the refname matches with one of the patterns
+ * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
+ * and so on, else return 0. Supports use of wild characters.
+ */
+static int match_name_as_path(const char **pattern, const char *refname)
+{
+ int namelen = strlen(refname);
+ for (; *pattern; pattern++) {
+ const char *p = *pattern;
+ int plen = strlen(p);
+
+ if ((plen = namelen)
+ !strncmp(refname, p, plen)
+ (refname[plen] == '\0' ||
+refname[plen] == '/' ||
+p[plen-1] == '/'))
+ return 1;
+ if (!wildmatch(p, refname, WM_PATHNAME, NULL))
+ return 1;
+ }
+ return 0;
+}
+
+/* Allocate space for a new refinfo and copy the objectname and flag to it */
+static struct refinfo *new_refinfo(const char *refname,
+ const unsigned char *objectname,
+ int flag)
+{
+ struct refinfo *ref = xcalloc(1, sizeof(struct refinfo));
+ ref-refname = xstrdup(refname);
+ hashcpy(ref-objectname, objectname);
+ ref-flag = flag;
+
+ return ref;
+}
+
+/*
* A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
*/
@@ -844,47 +881,20 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
{
struct grab_ref_cbdata *cb = cb_data;
struct refinfo *ref;
- int cnt;
if (flag REF_BAD_NAME) {
warning(ignoring ref with broken name %s, refname);
return 0;
}
- if (*cb-grab_pattern) {
- const char **pattern;
- int namelen = strlen(refname);
- for (pattern = cb-grab_pattern; *pattern; pattern++) {
- const char *p = *pattern;
- int plen = strlen(p);
-
- if ((plen = namelen)
- !strncmp(refname, p, plen)
- (refname[plen] == '\0' ||
-refname[plen] == '/' ||
-p[plen-1] == '/'))
- break;
- if (!wildmatch(p, refname, WM_PATHNAME, NULL))
- break;
- }
- if (!*pattern)
- return 0;
- }
+ if (*cb-grab_pattern !match_name_as_path(cb-grab_pattern, refname))
+ return 0;
- /*
-* We do not open the object yet; sort may only need refname
-* to do its job and the resulting list may yet to be pruned
-* by maxcount logic.
-*/
- ref = xcalloc(1, sizeof(*ref));
- ref-refname = xstrdup(refname);
- hashcpy(ref-objectname, sha1);
- ref-flag = flag;
+ ref = new_refinfo(refname, sha1, flag);
+
+ REALLOC_ARRAY(cb-grab_array, cb-grab_cnt + 1);
+ cb-grab_array[cb-grab_cnt++] = ref;
- cnt = cb-grab_cnt;
- REALLOC_ARRAY(cb-grab_array, cnt + 1);
- cb-grab_array[cnt++] = ref;
- cb-grab_cnt = cnt;
return 0;
}
--
2.4.1
--
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/RFC] send-email: allow multiple emails using --cc --to and --bcc
On Thu, May 28, 2015 at 11:26 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Thu, May 28, 2015 at 6:42 AM, Remi Lespinet
remi.lespi...@ensimag.grenoble-inp.fr wrote:
+ The format supported for email list is the following:
+ Foo f...@example.com, b...@example.com.
+ Please notice that the email list does not handle commas in
+ email names such as Foo, Bar foo...@example.com.
A few issues:
In order for this to format correctly in Asciidoc, the text needs to
be left-justified (just as was the line which you removed).
s/left-justified/unindented/
The sentence The format supported... seems superfluous. It's merely
repeating what is already clearly indicated by --bcc=address,...,
thus it could easily be dropped without harm.
Mention that commas in names are not currently handled is indeed a
s/Mention/Mentioning/
good idea, however, please tends to be an overused and wasted word
in documentation. More tersely:
Note: Addresses containing commas (Foo, Bar ...)
are not currently supported.
[...]
sub sanitize_address_list {
- return (map { sanitize_address($_) } @_);
+ my @addr_list = split_address_list(@_);
+ return (map { sanitize_address($_) } @addr_list);
}
Although, it was convenient from an implementation perspective to plop
the split_address_list() invocation into sanitize_address_list(), it
pollutes sanitize_address_list() by making it do something unrelated
to its purpose.
If you examine places where sanitize_address_list() is called, you will see:
validate_address_list(sanitize_address_list(@xx))
which clearly shows that sanitation and validation are distinct
Oops: s/sanitation/sanitization/
operations (and each function does only what its name implies). To
continue this idea, the splitting of addresses should be performed
distinctly from the other operations, in this order:
split - sanitize - validate
or:
validate_address_list(sanitize_address_list(
split_address_list(@xx))
That's pretty verbose, so introducing a new function to encapsulates
that behavior might be reasonable.
--
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/RFC 1/2] git-rebase -i: Add key word drop to remove a commit
Johannes Schindelin johannes.schinde...@gmx.de writes: Hi Stefan, On 2015-05-27 23:47, Stefan Beller wrote: On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano gits...@pobox.com wrote: Talking about ideas: I sometimes have the wrong branch checked out when doing a small fixup commit. So I want to drop that patch from the current branch and apply it to another branch. Maybe an instruction like cherry-pick-to-branch-(and-do-not-apply-here) would help me there. Oh, is it wish-anything time? *claps-his-hands* I would wish for a graphical tool which visualizes the commit graph in a visually pleasing manner, where I can select one or more commits and drop them onto a commit in the graph, and then it goes and magically cherry-picks-and-drops them. You need to argue a bit more to convince my students to schedule this for the end of their projects ;-). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 2/2] commit: fix ending newline for template files
Eric Sunshine sunsh...@sunshineco.com writes: Moreover, it lacks justification and explanation of why you consider the cleanup unnecessary. History [1] indicates that its application to -F but not -t was intentional. [1]: bc92377 (commit: fix ending newline for template files, 2015-05-26) Sorry, but the date of that commit seems to be too new to be considered history; I do not seem to have it, either. But I agree with you that I too failed to see why this change is necessary or desirable in the explanation in the proposed log message. -- 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 2/2] commit: fix ending newline for template files
On Thu, May 28, 2015 at 2:22 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: Moreover, it lacks justification and explanation of why you consider the cleanup unnecessary. History [1] indicates that its application to -F but not -t was intentional. [1]: bc92377 (commit: fix ending newline for template files, 2015-05-26) Sorry, but the date of that commit seems to be too new to be considered history; I do not seem to have it, either. Indeed, I somehow botched that. I meant: 8b1ae67 (Do not strip empty lines / trailing spaces from a commit message template, 2011-05-08) But I agree with you that I too failed to see why this change is necessary or desirable in the explanation in the proposed log message. -- 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] glossary: add remote, submodule, superproject
Stefan Beller sbel...@google.com writes: I am not sure if the discussion belongs into the glossary though. Probably not. Perhaps in tutorial. -- 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: [RFC/PATCH v2] create a skeleton for the command git rebase --status
Junio C Hamano gits...@pobox.com writes: If this topic enhances 'git status' with the in-progress rebase information, I'd view it as turning 'git status' from 'a more or less useless command during rebase' to 'a useful command'. For day-to-day operations, what we already have in status already qualifies as 'useful command' to me: $ git status rebase in progress; onto 7f9a792 You are currently rebasing branch 'master' on '7f9a792'. (fix conflicts and then run git rebase --continue) (use git rebase --skip to skip this patch) (use git rebase --abort to check out the original branch) I like the output of git status to be concise. OTOH, there are tons of information in .git/rebase-merge/ that could be displayed to the user. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 1/2] config: add options to list only variable names
Junio C Hamano gits...@pobox.com writes: The 'type' may also be shared between these two options, no? It would be logically consistent if you can say git config --bool --get-name-regexp '.*' 'no' to find all configuration variables that are set to 'false' in different spellings like '0', 'false', 'no', etc. And I suspect that the code already supports that. Not really. This of course inherits the same breakage from the existing --get-regexp code in that the value part is still matched as string. I am sure you could argue that but read the name of the last optional argument; it says value_REGEX and it is clearly about matching textually, and it may technically not be incorrect per-se, but I'd say it is merely a justification for a lazy implementation to defend the current less-than-useful behaviour. In any case, because it is still a textual match, user.name='Junio Hamano' matches with the above. The only reason why it does not barf is because it does not try to interpret and format that value as a boolean. So I would say that the feature supports [type] and [value_regex] to exactly the same degree as --get-regexp, i.e. with breakage. Which means we should document it the same way, even though both are equally broken. So that other people can later visit it and correct the issue for both options. -- 8 -- Subject: [PATCH] SQUASH??? The new option --get-name-regexp is a variant of --get-regexp; show them next to each other, and also document [type] and [value_regex] that the option seems to support. --- Documentation/git-config.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index b69c859..9fc78d8 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -15,8 +15,8 @@ SYNOPSIS 'git config' [file-option] [type] [-z|--null] --get name [value_regex] 'git config' [file-option] [type] [-z|--null] --get-all name [value_regex] 'git config' [file-option] [type] [-z|--null] --get-regexp name_regex [value_regex] +'git config' [file-option] [type] [-z|--null] --get-name-regexp name_regex [value_regex] 'git config' [file-option] [type] [-z|--null] --get-urlmatch name URL -'git config' [file-option] [-z|--null] --get-name-regexp name_regex 'git config' [file-option] --unset name [value_regex] 'git config' [file-option] --unset-all name [value_regex] 'git config' [file-option] --rename-section old_name new_name -- 2.4.2-521-g2db3d00 -- 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 1/3] t4150-am: refactor and clean common setup
On Thu, May 28, 2015 at 9:10 AM, Paul Tan pyoka...@gmail.com wrote:
Take these comments/suggestions with a pinch of salt because I'm not
that experienced with the code base as well ;-).
I agree with pretty much all of your review comments. See below for a
minor addenda.
On Wed, May 27, 2015 at 5:32 AM, Remi Lespinet
remi.lespi...@ensimag.grenoble-inp.fr wrote:
+setup_temporary_branch () {
Maybe name this checkout_temp_branch() or something, since it more or
less is just a wrapper around git-checkout.
checkout_temp_branch() doesn't give any indication that a new branch
is being created, so it may not be an improvement over
setup_temporary_branch(). A more apt name might be something like
new_temp_branch().
+ tmp_name=${2-temporary}
I don't think the quotes are required. Also, I don't feel good about
swapping the order of the arguments to git-checkout. (or making $2 an
optional argument). As the patch stands, if I see
setup_temporary_branch lorem
I will think: so we are creating a temporary branch lorem. But nope,
we are creating a temporary branch temporary that branches from
lorem. I don't think writing setup_temporary_branch temporary
lorem explicitly is that bad.
In fact, the second argument is never used in any of the three
patches, so it seems to be wasted functionality (at this time). If so,
then an even more appropriate name might be new_temp_branch_from().
Clearly, then:
new_temp_branch_from lorem
creates a throw-away branch based upon 'lorem'.
--
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 1/2] config: add options to list only variable names
SZEDER Gábor sze...@ira.uka.de writes: @@ -16,11 +16,12 @@ SYNOPSIS 'git config' [file-option] [type] [-z|--null] --get-all name [value_regex] 'git config' [file-option] [type] [-z|--null] --get-regexp name_regex [value_regex] 'git config' [file-option] [type] [-z|--null] --get-urlmatch name URL +'git config' [file-option] [-z|--null] --get-name-regexp name_regex ... At first I wondered why --get-name-regexp is needed, as (purely from the syntactic level) it appeared at the first glance the existing '--get-regexp' without 'value_regex' may be sufficient, until I read this: +--get-name-regexp:: + Like --get-regexp, but shows only matching variable names, not its + values. which makes it clear why it is needed. The distinction is purely about the output, i.e. the values are omitted. But then it makes me wonder why --get-name-regexp shouldn't optionally accept value_regex like --get-regexp does, allowing you to say list the variables that match this pattern whose values match this other pattern. I know it may be a feature that is unnecessary for your purpose, but from a cursory look of the patch text, you do not seem to be doing anything different between get-regexp and get-name-regexp codepaths other than flipping omit_values bit on, so it could be that the feature is already supported but forgot to document it, perhaps? The 'type' may also be shared between these two options, no? It would be logically consistent if you can say git config --bool --get-name-regexp '.*' 'no' to find all configuration variables that are set to 'false' in different spellings like '0', 'false', 'no', etc. And I suspect that the code already supports that. Tangentially related to the above issue, but git config --bool --get-regexp '.*' 'no' seems to be broken from that point of view. Instead of ignoring a non-bool string valued variables, it seems to barf upon seeing the first such variable. Interestingly, --get-name-regexp does not share that breakage ;-) Which we may want to fix, but not in the scope of this series. Hint, hint... -- 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 3/3] git-am: add am.threeWay config variable
Junio C Hamano gits...@pobox.com writes: I've noticed that in the block above that initializes all the variables, sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort= messageid= resolvemsg= resume= scissors= no_inbody_headers= git_apply_opt= committer_date_is_author_date= ignore_date= allow_rerere_autoupdate= gpg_sign_opt= threeway is not initialized at all, and thus I think running threeway=t git am blah will affect the behavior of git-am. Correct. I overlooked this when I originally did threeway. Perhaps a preparatory bugfix patch is warranted before this one. Also, I noticed that we do not check for --no-interactive, --no-signoff, --no-keep, --no-whitespace, etc. Even though adding support for them would not hurt, lack of these are OK, as long as we do not have configuration variables to tweak their defaults. I wouldn't worry about these issues, as the port to C will make them disappear anyway. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
BUG: For first push to a bare repo, using --tags prevents creation of master branch
Using git version 2.4.1.171.ga34f239, and gitolite v3.6.3 (b05c94ff). I apologize I don't know enough about the client vs server functions, during a git push, to know whether this is a git or gitolite issue. After cloning the public glibc repo, and creating a new bare repo on my local gitserver by adding it to gitolite.conf and pushing it: $ git push --set-upstream origin Counting objects: 339319, done. Delta compression using up to 10 threads. Compressing objects: 100% (55496/55496), done. Writing objects: 100% (339319/339319), 99.28 MiB | 22.67 MiB/s, done. Total 339319 (delta 283044), reused 331722 (delta 276505) To gitserver:glibctest * [new branch] master - master Branch master set up to track remote branch master from origin. **After resetting to before the last push**: (removing and re-cloning my clone of the public glibc repo, and resetting the repo on my local gitserver back to a new bare repo (removing the repo from gitolite.conf, pushing the conf, removing the repo directory in the gitolite user, re-adding the repo to gitolite.conf, and pushing the conf): $ git push --tags --set-upstream origin Counting objects: 382283, done. Delta compression using up to 10 threads. Compressing objects: 100% (62075/62075), done. Writing objects: 100% (382283/382283), 101.16 MiB | 19.64 MiB/s, done. Total 382283 (delta 316774), reused 376933 (delta 312407) To gitserver:glibctest * [new tag] cvs/ChangeLog - cvs/ChangeLog * [new tag] ... a bunch more ... * [new tag] glibc-2.9 - glibc-2.9 Note this time there is NO: * [new branch] master - master Branch master set up to track remote branch master from origin. Hence, my local gitserver has no master branch. If cloning in another location, I get a warning: remote HEAD refers to nonexistent ref, unable to checkout. that I don't get if I do a git push --set-upstream origin git push --tags instead. -- 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: BUG: For first push to a bare repo, using --tags prevents creation of master branch
Michael Darling darli...@gmail.com writes: $ git push --tags --set-upstream origin [...] Note this time there is NO: * [new branch] master - master Branch master set up to track remote branch master from origin. man git-push says: SYNOPSIS git push [...] [repository [refspec...]] [...] --tags All refs under refs/tags are pushed, in addition to refspecs explicitly listed on the command line. You did not provide any refspec (you provided repository but not refspec), hence --tags pushes only tags. So, this is the expected behavior. That said, we may want to add an option like --tags-also that would push tags _in addition_ to what would normally be pushed. --follow-tags does more or less this, though, but only for annotated tags pointing at a ref being pushed. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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/RFC 2/2] git rebase -i: Warn removed or dupplicated commits
Junio C Hamano gits...@pobox.com writes: I think there is a difference between (silently) accepting just to be lenient and documenting and advocating mixed case uses. Personally, I'd rather not to see gratuitous flexibility to allow the same thing spelled in 47 different ways for no good reason. It was more of a mistake on our part rather than actually wanting to document mixed case uses. In the v2 of the patch (not sent to the mailing list yet since we want to take into consideration the conclusion of this discussion before) it is entirely in lower case in both the documentation and the code while we silently allow upper and mixed case. -- 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 1/2] for-each-ref: re-structure code for moving to 'ref-filter'
On 05/26/2015 09:19 PM, Matthieu Moy wrote: Seconded. Some reasons/guide to split: * Split trivial and non-trivial stuff. I can quickly review a rename-only patch even if it's a bit long (essentially, I'll check that you did find-and-replace properly), but reviewing a mix of renames and actual code change is hard. * Split controversial and non-controversial stuff. For example, you changed the ordering of fields in a struct. Perhaps it was not a good idea. Perhaps it was a good idea, but then you want this reordering to be alone in its patch so that you can explain why it's a good idea in the commit message (you'll see me use the word why a lot when talking about commit messages; not a coincidence).u Since one of the patches is to restructure and rename 'for-each-ref', I thought It would be ideal to introduce the data structures within that patch, What do you think? * Split code movement and other stuff. For example extraction of match_name_as_path() would be trivial if made in its own patch. I'd also make a separate patch introduce the ref_list data-structure to introduce struct ref_list and basic helper functions (constructors, mutators, destructors). It will take time and may appear to be counter-productive at first, but you'll get used to it, and you'll end up being actually more productive this way (well, maybe not you but the set you + reviewers). Thanks for this. -- Regards, Karthik -- 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
[PATCH 1/4] read-cache.c: drop PROT_WRITE from mmap of index
Once upon a time, git's in-memory representation of a cache entry actually pointed to the mmap'd on-disk data. So in 520fc24 (Allow writing to the private index file mapping., 2005-04-26), we specified PROT_WRITE so that we could tweak the entries while we run (in our own MAP_PRIVATE copy-on-write version, of course). Later, 7a51ed6 (Make on-disk index representation separate from in-core one, 2008-01-14) stopped doing this; we copy the data into our in-core representation, and then drop the mmap immediately. We can therefore drop the PROT_WRITE flag. It's probably not hurting anything as it is, but it's potentially confusing. Note that we could also mark the mapping as const to verify that we never write to it. However, we don't typically do that for our other maps, as it then requires casting to munmap() it. Signed-off-by: Jeff King p...@peff.net --- This one obviously is not necessary for the rest of it, but just something I noticed while writing my response to you. But read to the end of the series; there might be a twist ending that brings it back! read-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/read-cache.c b/read-cache.c index 723d48d..5dee4e2 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1562,7 +1562,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist) if (mmap_size sizeof(struct cache_header) + 20) die(index file smaller than expected); - mmap = xmmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); + mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0); if (mmap == MAP_FAILED) die_errno(unable to map index file); close(fd); -- 2.4.2.668.gc3b1ade.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
[PATCH 2/4] config.c: fix mmap leak when writing config
We mmap the existing config file, but fail to unmap it if we
hit an error. The function already has a shared exit path,
so we can fix this by moving the mmap pointer to the
function scope and clearing it in the shared exit.
Signed-off-by: Jeff King p...@peff.net
---
config.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/config.c b/config.c
index ab46462..6917100 100644
--- a/config.c
+++ b/config.c
@@ -1939,6 +1939,8 @@ int git_config_set_multivar_in_file(const char
*config_filename,
int ret;
struct lock_file *lock = NULL;
char *filename_buf = NULL;
+ char *contents = NULL;
+ size_t contents_sz;
/* parse-key returns negative; flip the sign to feed exit(3) */
ret = 0 - git_config_parse_key(key, store.key, store.baselen);
@@ -1988,8 +1990,7 @@ int git_config_set_multivar_in_file(const char
*config_filename,
goto write_err_out;
} else {
struct stat st;
- char *contents;
- size_t contents_sz, copy_begin, copy_end;
+ size_t copy_begin, copy_end;
int i, new_line = 0;
if (value_regex == NULL)
@@ -2108,8 +2109,6 @@ int git_config_set_multivar_in_file(const char
*config_filename,
contents_sz - copy_begin)
contents_sz - copy_begin)
goto write_err_out;
-
- munmap(contents, contents_sz);
}
if (commit_lock_file(lock) 0) {
@@ -2135,6 +2134,8 @@ out_free:
if (lock)
rollback_lock_file(lock);
free(filename_buf);
+ if (contents)
+ munmap(contents, contents_sz);
return ret;
write_err_out:
--
2.4.2.668.gc3b1ade.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
[PATCH 3/4] config.c: avoid xmmap error messages
The config-writing code uses xmmap to map the existing
config file, which will die if the map fails. This has two
downsides:
1. The error message is not very helpful, as it lacks any
context about the file we are mapping:
$ mkdir foo
$ git config --file=foo some.key value
fatal: Out of memory? mmap failed: No such device
2. We normally do not die in this code path; instead, we'd
rather report the error and return an appropriate exit
status (which is part of the public interface
documented in git-config.1).
This patch introduces a gentle form of xmmap which lets us
produce our own error message. We do not want to use mmap
directly, because we would like to use the other
compatibility elements of xmmap (e.g., handling 0-length
maps portably).
The end result is:
$ git.compile config --file=foo some.key value
error: unable to mmap 'foo': No such device
$ echo $?
3
Signed-off-by: Jeff King p...@peff.net
---
config.c | 11 +--
git-compat-util.h | 1 +
sha1_file.c | 15 +++
3 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/config.c b/config.c
index 6917100..e7dc155 100644
--- a/config.c
+++ b/config.c
@@ -2053,8 +2053,15 @@ int git_config_set_multivar_in_file(const char
*config_filename,
fstat(in_fd, st);
contents_sz = xsize_t(st.st_size);
- contents = xmmap(NULL, contents_sz, PROT_READ,
- MAP_PRIVATE, in_fd, 0);
+ contents = xmmap_gently(NULL, contents_sz, PROT_READ,
+ MAP_PRIVATE, in_fd, 0);
+ if (contents == MAP_FAILED) {
+ error(unable to mmap '%s': %s,
+ config_filename, strerror(errno));
+ ret = CONFIG_INVALID_FILE;
+ contents = NULL;
+ goto out_free;
+ }
close(in_fd);
if (chmod(lock-filename.buf, st.st_mode 0) 0) {
diff --git a/git-compat-util.h b/git-compat-util.h
index 17584ad..0cc7ae8 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len);
extern void *xrealloc(void *ptr, size_t size);
extern void *xcalloc(size_t nmemb, size_t size);
extern void *xmmap(void *start, size_t length, int prot, int flags, int fd,
off_t offset);
+extern void *xmmap_gently(void *start, size_t length, int prot, int flags, int
fd, off_t offset);
extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
diff --git a/sha1_file.c b/sha1_file.c
index ccc6dac..73e0bc0 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -707,8 +707,8 @@ static void mmap_limit_check(size_t length)
(uintmax_t)length, (uintmax_t)limit);
}
-void *xmmap(void *start, size_t length,
- int prot, int flags, int fd, off_t offset)
+void *xmmap_gently(void *start, size_t length,
+ int prot, int flags, int fd, off_t offset)
{
void *ret;
@@ -719,12 +719,19 @@ void *xmmap(void *start, size_t length,
return NULL;
release_pack_memory(length);
ret = mmap(start, length, prot, flags, fd, offset);
- if (ret == MAP_FAILED)
- die_errno(Out of memory? mmap failed);
}
return ret;
}
+void *xmmap(void *start, size_t length,
+ int prot, int flags, int fd, off_t offset)
+{
+ void *ret = xmmap_gently(start, length, prot, flags, fd, offset);
+ if (ret == MAP_FAILED)
+ die_errno(Out of memory? mmap failed);
+ return ret;
+}
+
void close_pack_windows(struct packed_git *p)
{
while (p-windows) {
--
2.4.2.668.gc3b1ade.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
Re: Bug: .gitconfig folder
On Wed, May 27, 2015 at 03:24:47PM -0700, Stefan Beller wrote: What is our thinking for after-fact recovery attempts? Like we try the mmap first, if that fails we just use open to get the contents of the file. And when open fails, we can still print a nice error message? For config, I think we could just open and read the file in the first place. The data is not typically very big (and if you have a 3G config file and git barfs with out of memory, I can live with that). -Peff -- 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: Bug: .gitconfig folder
On Wed, May 27, 2015 at 03:38:12PM -0700, Junio C Hamano wrote: The patch was meant to be a tongue-in-cheek tangent that is a vast improvement for cases where we absolutely need to use mmap but does not help the OP at all ;-) I do not think there is any need for the config reader to read the existing file via mmap interface; just open it, strbuf_read() the whole thing (and complain when it cannot) and we should be ok. Or do we write back through the mmaped region or something? No, I think we must never do that in our code because our compat mmap implementation uses pread(). So all maps must be MAP_PRIVATE (and our compat mmap barfs if it is not). I started to go the strbuf_read() route, but it just felt so dirty to change the way the code works only to try to get a better error message. So here's my attempt at making it better while still using mmap. The end result is: $ mkdir foo $ git config --file=foo some.key value error: unable to mmap 'foo': Is a directory Having looked through the code, I think the _ideal_ way to implement it would actually be with read() and seek(). We read through the config once (with the normal parser, which wraps stdio) and mark the offsets of chunks we want to copy to the output. Then we mmap the original (under lock, at least, so it shouldn't be racy) and output the existing chunks and any new content in the appropriate order. So ideally writing each chunk would just be seek() and copy_fd(). But our offsets aren't quite perfect. In some cases we read backwards in our mmap to find the right cutoff point. I'm sure this is fixable given sufficient refactoring, but the config-writing code is such a tangled mess that I don't want to spend the time or risk the regressions. [1/4]: read-cache.c: drop PROT_WRITE from mmap of index [2/4]: config.c: fix mmap leak when writing config [3/4]: config.c: avoid xmmap error messages [4/4]: config.c: rewrite ENODEV into EISDIR when mmap fails -Peff -- 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
[PATCH 4/4] config.c: rewrite ENODEV into EISDIR when mmap fails
If we try to mmap a directory, we'll get ENODEV. This
translates to no such device for the user, which is not
very helpful. Since we've just fstat()'d the file, we can
easily check whether the problem was a directory to give a
better message.
Signed-off-by: Jeff King p...@peff.net
---
It feels a bit wrong to put this magic conversion here, and not in
xmmap. But of course xmmap does not have the stat information.
Which makes me wonder if we should provide an interface that will take
the whole struct stat rather than just the size. That's less flexible,
but in most cases, we're mapping the whole file (the packfiles are the
big exception, where we use a window).
We could also potentially drop some of the useless options. As of patch
1, all of our calls are PROT_READ. They must all be MAP_PRIVATE, or our
pread compatibility wrapper will fail, and we never use other flags. We
never request a specific address. And in a whole-file remap, the offset
will always be 0. So something like:
void *xmmap_file(int fd, struct stat *st);
would probably work. We could even do the fstat() on behalf of the
caller, though they need to know the length themselves. Maybe:
void *xmmap_file(int fd, size_t *len);
I dunno if it is worth it or not.
config.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/config.c b/config.c
index e7dc155..29fa012 100644
--- a/config.c
+++ b/config.c
@@ -2056,6 +2056,8 @@ int git_config_set_multivar_in_file(const char
*config_filename,
contents = xmmap_gently(NULL, contents_sz, PROT_READ,
MAP_PRIVATE, in_fd, 0);
if (contents == MAP_FAILED) {
+ if (errno == ENODEV S_ISDIR(st.st_mode))
+ errno = EISDIR;
error(unable to mmap '%s': %s,
config_filename, strerror(errno));
ret = CONFIG_INVALID_FILE;
--
2.4.2.668.gc3b1ade.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
Re: [PATCH 0/2] commit -t appends newline after template file
On Tue, May 26, 2015 at 8:15 AM, Patryk Obara patryk.ob...@gmail.com wrote: These are my first patches to git, so be extra pedantic during review, please. I noticed, that newline is appended, when I try to use template file - which is annoying if template ends with comment. I digged a bit and it turned out that: Hey, can anyone go through my commit and tell me if I need to improve anything or (maybe) accept it? -- | ← Ceci n'est pas une pipe Patryk Obara -- 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: Redirect git subcommand to itself?
On Wed, 27 May 2015 17:28:34 -0700
Stefan Beller sbel...@google.com wrote:
so I just run into this problem again (which happens to me maybe
twice a week): I want to do a git operations, so I type git into
my shell,
[...]
then I copy the whole operation git revert --abort in this case and
paste it to the shell and let go.
The result looks like
$ git git revert --abort
git: 'git' is not a git command. See 'git --help'.
[...]
I wonder if we want to make a git subcommand, which behaves exactly
the same as git itself?
Then git git git status would just return the same as git status.
In your ~/.whateverrc, put this:
git() {
while [ $# -gt 0 ]; do
test $1 != git break;
shift;
done;
command git $@;
}
This assumes a POSIX-compatible shell but I think you've got the idea.
(command is a builtin which forces interpreting the following word as
the name of an external program.)
--
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
[PATCH/RFC] send-email: allow multiple emails using --cc --to and --bcc
Hello, I've corrected an old patch from an Ensimag student. (http://thread.gmane.org/gmane.comp.version-control.git/228182). This patch allows multiple email addresses for options --cc, --to and --bcc. As said in the commit message, this patch doesn't handle commas in name, and the only possibility for using commas in name is to use the rfc2047 syntax: To: =?ISO-8859-1?Q?Ex=2C_ample?= t...@a.com I would like to add the possibility to use the following command lines: git send-email --to 'Ex, am ple t...@a.com' git send-email --to 'Ex, am ple t...@a.com' git send-email --to \Ex, am ple\ t...@a.com git send-email --to \Ex, am\ \ple\ t...@a.com Here are my questions : Is this a good idea to handle commas in name ? Do you have any suggestion about proposed syntaxes ? -- 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
[PATCH/RFC] send-email: allow multiple emails using --cc --to and --bcc
From: Jorge Juan Garcia Garcia jorge-juan.garcia-gar...@ensimag.imag.fr
Add the possibility to use a list of emails separated by commas
in flags --cc --to and --bcc instead of having to use one flag
per email address.
The use-case is to copy-paste a list of addresses from an email.
This change makes it so that we no longer need to cut the list.
The format of email list handled is basic for now:
$ git send-email --to='Foo f...@example.com, b...@example.com'
We thought it would be nice to have a first-step version which works
before handling more complex ones such as names with commas:
$ git send-email --to='Foo, Bar foo...@example.com'
This artificial limitation is imposed by 79ee555b (Check and document
the options to prevent mistakes, 2006-06-21).
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
Contributions-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
Documentation/git-send-email.txt | 23 ++
git-send-email.perl | 21 ++--
t/t9001-send-email.sh| 41
3 files changed, 58 insertions(+), 27 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 043f345..0aeddcb 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -49,17 +49,21 @@ Composing
of 'sendemail.annotate'. See the CONFIGURATION section for
'sendemail.multiEdit'.
---bcc=address::
+--bcc=[address,...]::
Specify a Bcc: value for each email. Default is the value of
'sendemail.bcc'.
-+
-The --bcc option must be repeated for each user you want on the bcc list.
+ The format supported for email list is the following:
+ Foo f...@example.com, b...@example.com.
+ Please notice that the email list does not handle commas in
+ email names such as Foo, Bar foo...@example.com.
---cc=address::
+--cc=[address,...]::
Specify a starting Cc: value for each email.
Default is the value of 'sendemail.cc'.
-+
-The --cc option must be repeated for each user you want on the cc list.
+ The format supported for email list is the following:
+ Foo f...@example.com, b...@example.com.
+ Please notice that the email list does not handle commas in
+ email names such as Foo, Bar foo...@example.com.
--compose::
Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
@@ -111,12 +115,15 @@ is not set, this will be prompted for.
is not set, this will be prompted for.
--to=address::
+--to=[address,...]::
Specify the primary recipient of the emails generated. Generally, this
will be the upstream maintainer of the project involved. Default is the
value of the 'sendemail.to' configuration value; if that is unspecified,
and --to-cmd is not specified, this will be prompted for.
-+
-The --to option must be repeated for each user you want on the to list.
+ The format supported for email list is the following:
+ Foo f...@example.com, b...@example.com.
+ Please notice that the email list does not handle commas in
+ email names such as Foo, Bar foo...@example.com.
--8bit-encoding=encoding::
When encountering a non-ASCII message or subject that does not
diff --git a/git-send-email.perl b/git-send-email.perl
index ffea500..409ff45 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -460,20 +460,6 @@ my ($repoauthor, $repocommitter);
($repoauthor) = Git::ident_person(@repo, 'author');
($repocommitter) = Git::ident_person(@repo, 'committer');
-# Verify the user input
-
-foreach my $entry (@initial_to) {
- die Comma in --to entry: $entry'\n unless $entry !~ m/,/;
-}
-
-foreach my $entry (@initial_cc) {
- die Comma in --cc entry: $entry'\n unless $entry !~ m/,/;
-}
-
-foreach my $entry (@bcclist) {
- die Comma in --bcclist entry: $entry'\n unless $entry !~ m/,/;
-}
-
sub parse_address_line {
if ($have_mail_address) {
return map { $_-format } Mail::Address-parse($_[0]);
@@ -1052,7 +1038,8 @@ sub sanitize_address {
}
sub sanitize_address_list {
- return (map { sanitize_address($_) } @_);
+ my @addr_list = split_address_list(@_);
+ return (map { sanitize_address($_) } @addr_list);
}
# Returns the local Fully Qualified Domain Name (FQDN) if available.
@@ -1193,6 +1180,10 @@ sub file_name_is_absolute {
return File::Spec::Functions::file_name_is_absolute($path);
}
+sub split_address_list {
+ return (map { split /\s*,\s*/, $_ } @_);
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index
Re: [PATCH v2 1/2] for-each-ref: re-structure code for moving to 'ref-filter'
Karthik Nayak karthik@gmail.com writes: On 05/26/2015 09:19 PM, Matthieu Moy wrote: Seconded. Some reasons/guide to split: * Split trivial and non-trivial stuff. I can quickly review a rename-only patch even if it's a bit long (essentially, I'll check that you did find-and-replace properly), but reviewing a mix of renames and actual code change is hard. * Split controversial and non-controversial stuff. For example, you changed the ordering of fields in a struct. Perhaps it was not a good idea. Perhaps it was a good idea, but then you want this reordering to be alone in its patch so that you can explain why it's a good idea in the commit message (you'll see me use the word why a lot when talking about commit messages; not a coincidence). Since one of the patches is to restructure and rename 'for-each-ref', I thought It would be ideal to introduce the data structures within that patch, What do you think? I don't have a universal answer: in general I prefer (let's say this list prefers) splitting as much as possible. It may make sense to group add data structure X with use data-structure X to make sure that functions you introduce have a caller. What's clear is that your PATCH 1/2 is not split enough. Just go through it, you'll see code movement (a pain to review in patch format), straigthforward renamings (easy to review as-is, but disturbs the reviewer when mixed with something else) and actual new code. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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/RFC] send-email: allow multiple emails using --cc --to and --bcc
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes:
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -519,6 +519,12 @@ Result: OK
EOF
+replace_variable_fields () {
+ sed -e s/^\(Date:\).*/\1 DATE-STRING/ \
+ -e s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/ \
+ -e s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/
+}
+
test_suppression () {
git send-email \
--dry-run \
@@ -526,10 +532,7 @@ test_suppression () {
--from=Example f...@example.com \
--to=t...@example.com \
--smtp-server relay.example.com \
- $patches |
- sed -e s/^\(Date:\).*/\1 DATE-STRING/ \
- -e s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/ \
- -e s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/ \
+ $patches | replace_variable_fields \
I wouldn't insist on that, but this change would be better done in a
separate, preparatory patch (that would be PATCH 1/2, and the actual
code would be PATCH 2/2).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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/RFC] create a skeleton for the command git rebase --status
Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr writes:
It is an almost empty code I send to validate the global architecture
of this command. I choose to write
Avoid personal wording (I choose to write ... because - The code is
written ... because). What matters in the commit message is the reason
for the choice, not who made it.
+BUILTIN_OBJS += builtin/rebase--status--helper.o
No builtin/rebase--status--helper.c in your patch, is it intended?
+status)
+ git rebase--status--helper
+ die
die is used to exit with an error code ($? != 0). You just mean exit
$? here, to exit with the same code as the helper.
And actually, you don't need to keep your script alive while the helper
is ran, so you can write
exec git rebase--status--helper
--- /dev/null
+++ b/rebase--status.c
@@ -0,0 +1,6 @@
+#include rebase--status.h
+
+int rebase_status(){
+ printf(Rebase in progress\n);
... or not.
Avoid leaving incorrect things like this in intermediate steps, even if
you're going to fix them eventually. It's too easy to forget the actual
fix and leave a Rebase in progress message even when there's no rebase
in progress. And reviewers may get confused.
I'd have written stg like
printf(git rebase --status is not yet implemented);
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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
wish dies when ibus is restarted
Hello. Recently, the guys from Google have to deal with an issue in Android Studio where the user can no longer type in the IDE. Their workaround is to restart ibus via ibus restart, which works more or less [1]. As a side effect I noticed that wish and therefore gitk and git-gui die as soon as I execute the command. I want to let you know about this in case you can do anything about this. Best, Tobias __ [1] http://tools.android.com/knownissues/ibus -- 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/RFC] create a skeleton for the command git rebase --status
Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr writes: It is an almost empty code I send to validate the global architecture of this command. One more note: I know what git rebase --status is about because I'm the one who suggested it and I read your previous message, but not everybody is me, and most people would appreciate a brief explanation of what git rebase --status is before knowing how you're starting the work. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 1.5/2] config: add options to list only variable names
On Wed, May 27, 2015 at 10:11 PM, SZEDER Gábor sze...@ira.uka.de wrote: Recenty I created a multi-line branch description with '.' and '=' characters on one of the lines, and noticed that fragments of that line show up when completing set variable names for 'git config', e.g.: $ git config --get branch.b.description Branch description to fool the completion script with a second line containing dot . and equals = characters. $ git config --unset TAB ... second line containing dot . and equals ... The completion script runs 'git config --list' and processes its output to strip the values and keep only the variable names. It does so by looking for lines containing '.' and '=' and outputting everything before the '=', which was fooled by my multi-line branch description. A similar issue exists with aliases and pretty format aliases with multi-line values, but in that case 'git config --get-regexp' is run and subsequent lines don't have to contain either '.' or '=' to fool the completion script. Though 'git config' can produce null-terminated output for newline-safe parsing, that's of no use in this case, becase we can't cope with nulls in the shell. Help the completion script by introducing the '--list-names' and '--get-names-regexp' options, the names-only equivalents of '--list' and '--get-regexp', so it doesn't have to separate variable names from their values anymore. Why don't you just add a '--name-only' option that can be used only with '--list' and '--get-regexp'? Like: 'git config' [file-option] [-z|--null] [--name-only] --get-regexp name_regex and 'git config' [file-option] [-z|--null] [--name-only] -l | --list ? It seems to me that it would reduce the number of options, and later if we want to pass a format we could have maybe: 'git config' [file-option] [-z|--null] [--name-only | --format=format] --get-regexp name_regex and 'git config' [file-option] [-z|--null] [--name-only | --format=format] -l | --list Thanks, Christian. -- 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: Redirect git subcommand to itself?
Stefan Beller sbel...@google.com writes: so I just run into this problem again (which happens to me maybe twice a week): I want to do a git operations, so I type git into my shell, and then [...] I copy the whole operation git revert --abort in this case and paste it to the shell On my side, that wouldn't be twice a week, but I often do the same mistake. I find your idea for a solution conceptually ugly but practically very convenient ;-), so I'd be in favor of doing it. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 1/3] t4150-am: refactor and clean common setup
Hi,
Take these comments/suggestions with a pinch of salt because I'm not
that experienced with the code base as well ;-).
On Wed, May 27, 2015 at 5:32 AM, Remi Lespinet
remi.lespi...@ensimag.grenoble-inp.fr wrote:
Add new functions to keep the setup cleaner:
- setup_temporary_branch: creates a new branch, check it out
and automatically delete it after the test is over
Agreed. This removes a lot of boilerplate from the tests. Another
positive effect is that we can be sure that any commits made on that
throwaway branch will not affect the other parts of the test suite,
which makes understanding and editing the test suite much easier.
- setup_fixed_branch: creates a fixed branch, which can be re-used
in later tests
Looking at the patch, I see that setup_fixed_branch() is only used in
2 places, so I don't think it is a common pattern that would require
its own function. Also, see below.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
t/t4150-am.sh | 138
--
1 file changed, 47 insertions(+), 91 deletions(-)
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 306e6f3..8370951 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -4,6 +4,20 @@ test_description='git am running'
. ./test-lib.sh
+setup_temporary_branch () {
Maybe name this checkout_temp_branch() or something, since it more or
less is just a wrapper around git-checkout.
+ tmp_name=${2-temporary}
I don't think the quotes are required. Also, I don't feel good about
swapping the order of the arguments to git-checkout. (or making $2 an
optional argument). As the patch stands, if I see
setup_temporary_branch lorem
I will think: so we are creating a temporary branch lorem. But nope,
we are creating a temporary branch temporary that branches from
lorem. I don't think writing setup_temporary_branch temporary
lorem explicitly is that bad.
This is just personal preference though.
+ git reset --hard
+ rm -fr .git/rebase-apply
+ test_when_finished git checkout $1 git branch -D $tmp_name
I think you should use git checkout -f $1 as if the working tree is
dirty the test will fail at cleanup with no error message at all,
which is annoying for debugging. Furthermore, the test_when_finished
should be shifted below the git checkout -b below, as git branch -D
will fail if the branch does not exist.
+ git checkout -b $tmp_name $1
If you use git checkout -f here then there's no need for the git reset
--hard above.
+}
+
+setup_fixed_branch () {
+ git reset --hard
+ rm -fr .git/rebase-apply
+ git checkout -b $1 $2
Again, by using git checkout -f we can drop the git reset --hard. But
that reduces the function to 2 lines, and thus I don't think that this
usage pattern needs its own function.
+}
+
test_expect_success 'setup: messages' '
cat msg -\EOF
second
@@ -143,9 +157,7 @@ test_expect_success setup '
'
I haven't looked at the rest of the patch in detail because I'm not
that well-acquainted with t4150 yet, but it looks okay.
Thanks,
Paul
--
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
[PATCH/RFC] create a skeleton for the command git rebase --status
It is an almost empty code I send to validate the global architecture
of this command. I choose to write it in C because git status is
already in C and it seems that it is the current tendency to port
shell code to C. Moreover I would like to use code from wt_status to
implement this functionnality. I wrote a helper that I call from shell
script, as it is made in bisect (bisect--helper.c).
Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr
---
Makefile | 2 ++
builtin.h| 1 +
git-rebase.sh| 7 ++-
git.c| 1 +
rebase--status.c | 6 ++
rebase--status.h | 7 +++
6 files changed, 23 insertions(+), 1 deletion(-)
create mode 100644 rebase--status.c
create mode 100644 rebase--status.h
diff --git a/Makefile b/Makefile
index e0caec3..e3b3e63 100644
--- a/Makefile
+++ b/Makefile
@@ -853,6 +853,7 @@ LIB_OBJS += prompt.o
LIB_OBJS += quote.o
LIB_OBJS += reachable.o
LIB_OBJS += read-cache.o
+LIB_OBJS += rebase--status.o
LIB_OBJS += reflog-walk.o
LIB_OBJS += refs.o
LIB_OBJS += remote.o
@@ -969,6 +970,7 @@ BUILTIN_OBJS += builtin/prune-packed.o
BUILTIN_OBJS += builtin/prune.o
BUILTIN_OBJS += builtin/push.o
BUILTIN_OBJS += builtin/read-tree.o
+BUILTIN_OBJS += builtin/rebase--status--helper.o
BUILTIN_OBJS += builtin/receive-pack.o
BUILTIN_OBJS += builtin/reflog.o
BUILTIN_OBJS += builtin/remote.o
diff --git a/builtin.h b/builtin.h
index c47c110..5071a08 100644
--- a/builtin.h
+++ b/builtin.h
@@ -99,6 +99,7 @@ extern int cmd_prune(int argc, const char **argv, const char
*prefix);
extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
extern int cmd_push(int argc, const char **argv, const char *prefix);
extern int cmd_read_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_rebase_status__helper(int argc, const char **argv, const char
*prefix);
extern int cmd_receive_pack(int argc, const char **argv, const char *prefix);
extern int cmd_reflog(int argc, const char **argv, const char *prefix);
extern int cmd_remote(int argc, const char **argv, const char *prefix);
diff --git a/git-rebase.sh b/git-rebase.sh
index 47ca3b9..8454071 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -43,6 +43,7 @@ continue! continue
abort! abort and check out the original branch
skip! skip current patch and continue
edit-todo! edit the todo list during an interactive rebase
+status!show the status of the current rebase
. git-sh-setup
. git-sh-i18n
@@ -238,7 +239,7 @@ do
--verify)
ok_to_skip_pre_rebase=
;;
- --continue|--skip|--abort|--edit-todo)
+ --continue|--skip|--abort|--edit-todo|--status)
test $total_argc -eq 2 || usage
action=${1##--}
;;
@@ -401,6 +402,10 @@ abort)
edit-todo)
run_specific_rebase
;;
+status)
+ git rebase--status--helper
+ die
+ ;;
esac
# Make sure no rebase is in progress
diff --git a/git.c b/git.c
index 9efd1a3..3ebc144 100644
--- a/git.c
+++ b/git.c
@@ -410,6 +410,7 @@ static struct cmd_struct commands[] = {
{ prune-packed, cmd_prune_packed, RUN_SETUP },
{ push, cmd_push, RUN_SETUP },
{ read-tree, cmd_read_tree, RUN_SETUP },
+ { rebase--status--helper, cmd_rebase_status__helper, RUN_SETUP },
{ receive-pack, cmd_receive_pack },
{ reflog, cmd_reflog, RUN_SETUP },
{ remote, cmd_remote, RUN_SETUP },
diff --git a/rebase--status.c b/rebase--status.c
new file mode 100644
index 000..d67af52
--- /dev/null
+++ b/rebase--status.c
@@ -0,0 +1,6 @@
+#include rebase--status.h
+
+int rebase_status(){
+ printf(Rebase in progress\n);
+ return 0;
+}
diff --git a/rebase--status.h b/rebase--status.h
new file mode 100644
index 000..17d22a1
--- /dev/null
+++ b/rebase--status.h
@@ -0,0 +1,7 @@
+#ifndef REBASE__STATUS_H
+#define REBASE__STATUS_H
+
+
+extern int rebase_status();
+
+#endif
--
2.0.5.5.g1d968ca.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
[PATCH v2 0/2] config: list only variable names for completion
Fixed misspelled option names in docs and in commit message, incorporated Peff's suggestions, and renamed 'show_only_keys' to 'omit_values' in 1/2. Only minor tweaking in 2/2's commit message. SZEDER Gábor (2): config: add options to list only variable names completion: use new 'git config' options to reliably list variable names Documentation/git-config.txt | 12 ++-- builtin/config.c | 17 ++--- contrib/completion/git-completion.bash | 19 +-- t/t1300-repo-config.sh | 22 ++ 4 files changed, 51 insertions(+), 19 deletions(-) -- 2.4.2.349.g6883b65 -- 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
[PATCH v2 1/2] config: add options to list only variable names
Recenty I created a multi-line branch description with '.' and '='
characters on one of the lines, and noticed that fragments of that line
show up when completing set variable names for 'git config', e.g.:
$ git config --get branch.b.description
Branch description to fool the completion script with a
second line containing dot . and equals = characters.
$ git config --unset TAB
...
second line containing dot . and equals
...
The completion script runs 'git config --list' and processes its output to
strip the values and keep only the variable names. It does so by looking
for lines containing '.' and '=' and outputting everything before the '=',
which was fooled by my multi-line branch description.
A similar issue exists with aliases and pretty format aliases with
multi-line values, but in that case 'git config --get-regexp' is run and
subsequent lines don't have to contain either '.' or '=' to fool the
completion script.
Though 'git config' can produce null-terminated output for newline-safe
parsing, that's of no use in this case, becase we can't cope with nulls in
the shell.
Help the completion script by introducing the '--list-names' and
'--get-name-regexp' options, the names-only equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
Documentation/git-config.txt | 12 ++--
builtin/config.c | 17 ++---
contrib/completion/git-completion.bash | 4 ++--
t/t1300-repo-config.sh | 22 ++
4 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 02ec096faa..b69c8592ac 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -16,11 +16,12 @@ SYNOPSIS
'git config' [file-option] [type] [-z|--null] --get-all name [value_regex]
'git config' [file-option] [type] [-z|--null] --get-regexp name_regex
[value_regex]
'git config' [file-option] [type] [-z|--null] --get-urlmatch name URL
+'git config' [file-option] [-z|--null] --get-name-regexp name_regex
'git config' [file-option] --unset name [value_regex]
'git config' [file-option] --unset-all name [value_regex]
'git config' [file-option] --rename-section old_name new_name
'git config' [file-option] --remove-section name
-'git config' [file-option] [-z|--null] -l | --list
+'git config' [file-option] [-z|--null] -l | --list | --list-names
'git config' [file-option] --get-color name [default]
'git config' [file-option] --get-colorbool name [stdout-is-tty]
'git config' [file-option] -e | --edit
@@ -96,6 +97,10 @@ OPTIONS
in which section and variable names are lowercased, but subsection
names are not.
+--get-name-regexp::
+ Like --get-regexp, but shows only matching variable names, not its
+ values.
+
--get-urlmatch name URL::
When given a two-part name section.key, the value for
section.url.key whose url part matches the best to the
@@ -159,7 +164,10 @@ See also FILES.
-l::
--list::
- List all variables set in config file.
+ List all variables set in config file, along with their values.
+
+--list-names::
+ List the names of all variables set in config file.
--bool::
'git config' will ensure that the output is true or false
diff --git a/builtin/config.c b/builtin/config.c
index 7188405f7e..c23f329b00 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -13,6 +13,7 @@ static char *key;
static regex_t *key_regexp;
static regex_t *regexp;
static int show_keys;
+static int omit_values;
static int use_key_regexp;
static int do_all;
static int do_not_match;
@@ -43,6 +44,8 @@ static int respect_includes = -1;
#define ACTION_GET_COLOR (113)
#define ACTION_GET_COLORBOOL (114)
#define ACTION_GET_URLMATCH (115)
+#define ACTION_LIST_NAMES (116)
+#define ACTION_GET_NAME_REGEXP (117)
#define TYPE_BOOL (10)
#define TYPE_INT (11)
@@ -60,6 +63,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, get, actions, N_(get value: name [value-regex]),
ACTION_GET),
OPT_BIT(0, get-all, actions, N_(get all values: key
[value-regex]), ACTION_GET_ALL),
OPT_BIT(0, get-regexp, actions, N_(get values for regexp:
name-regex [value-regex]), ACTION_GET_REGEXP),
+ OPT_BIT(0, get-name-regexp, actions, N_(get names for regexp:
name-regex), ACTION_GET_NAME_REGEXP),
OPT_BIT(0, get-urlmatch, actions, N_(get value specific for the
URL: section[.var] URL), ACTION_GET_URLMATCH),
OPT_BIT(0, replace-all, actions, N_(replace all matching variables:
name value [value_regex]), ACTION_REPLACE_ALL),
OPT_BIT(0, add, actions, N_(add a new variable: name value),
ACTION_ADD),
@@ -68,6 +72,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, rename-section, actions, N_(rename section: old-name
new-name),
[PATCH v2 2/2] completion: use new 'git config' options to reliably list variable names
List all set config variable names with 'git config --list-names' instead
of '--list' and post processing. Similarly, use 'git config
--get-name-regexp' instead of '--get-regexp' to get config variables in a
given section.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-completion.bash | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6abbd564b6..121aa31342 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -744,9 +744,8 @@ __git_compute_porcelain_commands ()
__git_get_config_variables ()
{
local section=$1 i IFS=$'\n'
- for i in $(git --git-dir=$(__gitdir) config --get-regexp
^$section\..* 2/dev/null); do
- i=${i#$section.}
- echo ${i/ */}
+ for i in $(git --git-dir=$(__gitdir) config --get-name-regexp
^$section\..* 2/dev/null); do
+ echo ${i#$section.}
done
}
@@ -1774,15 +1773,7 @@ __git_config_get_set_variables ()
c=$((--c))
done
- git --git-dir=$(__gitdir) config $config_file --list 2/dev/null |
- while read -r line
- do
- case $line in
- *.*=*)
- echo ${line/=*/}
- ;;
- esac
- done
+ git --git-dir=$(__gitdir) config $config_file --list-names 2/dev/null
}
_git_config ()
--
2.4.2.349.g6883b65
--
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 3/3] git-am: add am.threeWay config variable
Hi, On Wed, May 27, 2015 at 5:32 AM, Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr wrote: Add the am.threeWay configuration variable to use the -3 or --3way option of git am by default. When am.threeway is set and not desired for a specific git am command, the --no-3way option can be used to override it. Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr --- Even if git am will be rewritten soon, the code that will have to be ported is not the most important part of the patch and the tests and documentation parts can be reused. Yup, there's no problem on my side. Documentation/config.txt | 7 +++ Documentation/git-am.txt | 6 -- git-am.sh| 7 +++ t/t4150-am.sh| 15 +++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index d44bc85..8e42752 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -769,6 +769,13 @@ am.keepcr:: by giving '--no-keep-cr' from the command line. See linkgit:git-am[1], linkgit:git-mailsplit[1]. +am.threeWay:: + If true, git-am will fall back on 3-way merge when the patch + cannot be applied cleanly, in the same way as the '-3' or + '--3-way' option. Can be overridden by giving '--no-3-way' + from the command line. + See linkgit:git-am[1]. + Maybe we should start by mentioning the default behavior for git-am. Something like, By default, git-am will fail if the patch does not apply cleanly. When set to true, this setting tells git-am to fall back on 3-way merge if the patch records the identity of blobs it is supposed to apply to and we have those blobs available locally (equivalent to giving the --3way option from the command line). (I stole most of the text from the git-am and git-config documentation) apply.ignoreWhitespace:: When set to 'change', tells 'git apply' to ignore changes in whitespace, in the same way as the '--ignore-space-change' diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt index 0d8ba48..3190c05 100644 --- a/Documentation/git-am.txt +++ b/Documentation/git-am.txt @@ -89,11 +89,13 @@ default. You can use `--no-utf8` to override this. linkgit:git-mailinfo[1]). -3:: ---3way:: +--[no-]3way:: There's no need to mention --no-3way, it's assumed that most of the options have the equivalent --no-option because it is implemented automatically by the option parser. When the patch does not apply cleanly, fall back on 3-way merge if the patch records the identity of blobs it is supposed to apply to and we have those blobs - available locally. + available locally. `am.threeWay` configuration variable + can be used to specify the default behaviour. `--no-3way` + is useful to override `am.threeWay`. Usually configuration settings are mentioned in a separate section in the documentation CONFIGURATION (or not mentioned at all). Also, there's no need to mention that --no-3way can be used to mention the configuration, as its usual (and expected) that the configuration value sets the default behavior, and the command-line switch can override i. --ignore-space-change:: --ignore-whitespace:: diff --git a/git-am.sh b/git-am.sh index 761befb..781507c 100755 --- a/git-am.sh +++ b/git-am.sh @@ -389,6 +389,11 @@ then keepcr=t fi +if test $(git config --bool --get am.threeWay) = true +then +threeway=t +fi + while test $# != 0 do case $1 in @@ -400,6 +405,8 @@ it will be removed. Please do not use it anymore. ;; -3|--3way) threeway=t ;; + --no-3way) + threeway=f ;; Okay. This patch on its own, though, does not handle the case where: 1. am.threeWay is set to true 2. we passed --no-3way in the command-line 3. git-am failed to apply a patch and quit 4. We git-am --skip or git-am --continue. When continuing, git-am will not remember the --no-3way (unless we provide it again on the command line) They key is to tweak the following lines where the threeway setting is loaded: if test $(cat $dotest/threeway) = t then threeway=t fi To something like if test $(cat $dotest/threeway) = t then threeway=t else threeway=f fi -s|--signoff) sign=t ;; -u|--utf8) diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 8f85098..e16ef0e 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -288,6 +288,21 @@ test_expect_success 'am -3 falls back to 3-way merge' ' git diff --exit-code lorem ' +test_expect_success 'am with config am.threeWay falls back to 3-way merge' ' + setup_temporary_branch lorem2 + test_config am.threeWay 1 + git am lorem-move.patch + test_path_is_missing .git/rebase-apply + git diff
[RFC/PATCH v2] create a skeleton for the command git rebase --status
Preparatory commit for a git rebase --status command. This command
will indicate the state of the process in the rebase, and the reason
why it stopped.
Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr
---
The observations from Matthieu Moy have been taken into account
It is an almost empty code sent to validate the global architecture of
this command. It is written in C because git status is already in C
and it seems that it is the current tendency to port shell code to
C. Moreover will likely use code from wt_status to implement this
functionnality. The command calls a helper from a shell script, as it
is made in bisect (bisect--helper.c).
Makefile | 2 ++
builtin.h| 1 +
builtin/rebase--status--helper.c | 23 +++
git-rebase.sh| 6 +-
git.c| 1 +
rebase--status.c | 6 ++
rebase--status.h | 7 +++
7 files changed, 45 insertions(+), 1 deletion(-)
create mode 100644 builtin/rebase--status--helper.c
create mode 100644 rebase--status.c
create mode 100644 rebase--status.h
diff --git a/Makefile b/Makefile
index e0caec3..e3b3e63 100644
--- a/Makefile
+++ b/Makefile
@@ -853,6 +853,7 @@ LIB_OBJS += prompt.o
LIB_OBJS += quote.o
LIB_OBJS += reachable.o
LIB_OBJS += read-cache.o
+LIB_OBJS += rebase--status.o
LIB_OBJS += reflog-walk.o
LIB_OBJS += refs.o
LIB_OBJS += remote.o
@@ -969,6 +970,7 @@ BUILTIN_OBJS += builtin/prune-packed.o
BUILTIN_OBJS += builtin/prune.o
BUILTIN_OBJS += builtin/push.o
BUILTIN_OBJS += builtin/read-tree.o
+BUILTIN_OBJS += builtin/rebase--status--helper.o
BUILTIN_OBJS += builtin/receive-pack.o
BUILTIN_OBJS += builtin/reflog.o
BUILTIN_OBJS += builtin/remote.o
diff --git a/builtin.h b/builtin.h
index c47c110..5071a08 100644
--- a/builtin.h
+++ b/builtin.h
@@ -99,6 +99,7 @@ extern int cmd_prune(int argc, const char **argv, const char
*prefix);
extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
extern int cmd_push(int argc, const char **argv, const char *prefix);
extern int cmd_read_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_rebase_status__helper(int argc, const char **argv, const char
*prefix);
extern int cmd_receive_pack(int argc, const char **argv, const char *prefix);
extern int cmd_reflog(int argc, const char **argv, const char *prefix);
extern int cmd_remote(int argc, const char **argv, const char *prefix);
diff --git a/builtin/rebase--status--helper.c b/builtin/rebase--status--helper.c
new file mode 100644
index 000..efda29c
--- /dev/null
+++ b/builtin/rebase--status--helper.c
@@ -0,0 +1,23 @@
+#include builtin.h
+#include cache.h
+#include parse-options.h
+#include rebase--status.h
+
+static const char * const git_rebase_status_helper_usage[] = {
+ N_(git rebase--status--helper),
+ NULL
+};
+
+int cmd_rebase_status__helper(int argc, const char **argv, const char *prefix)
+{
+ struct option options[] = {
+
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+git_rebase_status_helper_usage, 0);
+
+
+ /* next-all */
+ return rebase_status();
+}
diff --git a/git-rebase.sh b/git-rebase.sh
index 47ca3b9..4e1f3e1 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -43,6 +43,7 @@ continue! continue
abort! abort and check out the original branch
skip! skip current patch and continue
edit-todo! edit the todo list during an interactive rebase
+status!show the status of the current rebase
. git-sh-setup
. git-sh-i18n
@@ -238,7 +239,7 @@ do
--verify)
ok_to_skip_pre_rebase=
;;
- --continue|--skip|--abort|--edit-todo)
+ --continue|--skip|--abort|--edit-todo|--status)
test $total_argc -eq 2 || usage
action=${1##--}
;;
@@ -401,6 +402,9 @@ abort)
edit-todo)
run_specific_rebase
;;
+status)
+ exec git rebase--status--helper
+ ;;
esac
# Make sure no rebase is in progress
diff --git a/git.c b/git.c
index 9efd1a3..3ebc144 100644
--- a/git.c
+++ b/git.c
@@ -410,6 +410,7 @@ static struct cmd_struct commands[] = {
{ prune-packed, cmd_prune_packed, RUN_SETUP },
{ push, cmd_push, RUN_SETUP },
{ read-tree, cmd_read_tree, RUN_SETUP },
+ { rebase--status--helper, cmd_rebase_status__helper, RUN_SETUP },
{ receive-pack, cmd_receive_pack },
{ reflog, cmd_reflog, RUN_SETUP },
{ remote, cmd_remote, RUN_SETUP },
diff --git a/rebase--status.c b/rebase--status.c
new file mode 100644
index 000..1962349
--- /dev/null
+++ b/rebase--status.c
@@ -0,0 +1,6 @@
+#include rebase--status.h
+
+int rebase_status(){
+ printf(git rebase --status is not yet implemented\n);
+ return 0;
+}
diff --git a/rebase--status.h
Re: [PATCH 1/2] t750*: make tests for commit messages more pedantic
On Tue, May 26, 2015 at 2:15 AM, Patryk Obara patryk.ob...@gmail.com wrote:
Currently messages are compared with --pretty=format:%s%b which does
not retain raw format of commit message. In result it's not clear what
part of expected commit msg is subject and what part is body. Also, it's
impossible to test if messages with multiple lines are handled
correctly, which may be significant when using nondefault --cleanup.
Makes sense.
Change commit_msg_is function to use raw message format in log and
interpret escaped sequences in expected message. This way it's possible
to test exactly what commit message text was saved.
These changes would be less daunting to review if split into multiple
patches; one per logical change. So, for instance, patch 1 would make
this change and adjust tests accordingly.
Add test to verify, that no additional content is appended to template
message, which uncovers tiny bug in --status handling - new line is
always appended before status message. If template file ended with
newline (which is default for many popular text editors, e.g. vim)
then blank line appears before status (which is very annoying when
template ends with line starting with '#'). On the other hand, this
newline needs to be appended if template file didn't end with newline
(which is default for e.g. emacs) - otherwise first line of status
message may be not cleaned up.
This could be patch 2.
Add explicit test to verify if \n is kept unexpanded in commit message -
this used to be part of unrelated template test.
And patch 3, and so on...
Modify add-content-and-comment fake editor to include both comments and
whitespace, so --cleanup=whitespace is now actually tested.
Modify expected value of test cleanup commit messages (t7502), which
shouldn't be passing, because template and logfiles are unnecessarily
stripped before placing them into editor.
Your cover letter correctly states that with this patch is applied, a
number of tests fail. Tests which are expected to fail should be
declared test_expect_failure rather than test_expect_success. The
patch which fixes the failures should flip them to
test_expect_success.
Signed-off-by: Patryk Obara patryk.ob...@gmail.com
More below...
---
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 116885a..fd1bf71 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -13,8 +13,8 @@ commit_msg_is () {
expect=commit_msg_is.expect
actual=commit_msg_is.actual
- printf %s $(git log --pretty=format:%s%b -1) $actual
- printf %s $1 $expect
+ git log --pretty=format:%B -1 $actual
+ printf %b $1 $expect
test_i18ncmp $expect $actual
}
@@ -329,4 +329,47 @@ test_expect_success 'invalid message options when using
--fixup' '
test_must_fail git commit --fixup HEAD~1 -F log
'
+test_expect_success 'no blank lines appended after template with --status' '
+ echo template line $TEMPLATE
Style: Modern code omits the space after the redirection operator
($TEMPLATE), however, it's also important to match existing style.
Unfortunately, this file has an equal mixture of both 'blap' and '
blap', so it's difficult to know which style to match. As this is new
code, it'd probably be best to omit the space.
+ echo changes foo
+ git add foo
+ test_set_editor $TEST_DIRECTORY/t7500/add-content
+ git commit -e -t $TEMPLATE --status
+ commit_msg_is template line\ncommit message\n
+'
+
+test_expect_success 'template without newline before eof should work with
--status' '
It's not clear what should work means. I suppose you mean that the
end result should have exactly one newline after the template. Perhaps
the test title could indicate the intent more clearly.
+ printf %s template line $TEMPLATE
+ echo changes foo
+ git add foo
+ test_set_editor $TEST_DIRECTORY/t7500/add-content
+ git commit -e -t $TEMPLATE --status
+ commit_msg_is template line\ncommit message\n
+'
+
+test_expect_success 'logfile without newline before eof should work with
--status' '
Ditto: Unclear should work
+ printf %s log line log-file
+ echo changes foo
+ git add foo
+ test_set_editor $TEST_DIRECTORY/t7500/add-content
+ git commit -e -F log-file --status
+ commit_msg_is log line\ncommit message\n
+'
test_done
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 051489e..d2203ed 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -8,11 +8,12 @@ commit_msg_is () {
expect=commit_msg_is.expect
actual=commit_msg_is.actual
- printf %s $(git log --pretty=format:%s%b -1) $actual
- printf %s $1 $expect
- test_i18ncmp $expect $actual
+ git log --pretty=format:%B -1 $actual
+ printf %b $1 $expect
+ test_i18ncmp $expect $actual
}
+
Sneaking in unnecessary whitespace change.
#
Re: [PATCH 2/2] commit: fix ending newline for template files
On Tue, May 26, 2015 at 2:15 AM, Patryk Obara patryk.ob...@gmail.com wrote:
git-commit with -t or -F -e uses content of user-supplied file as
initial value for commit msg in editor. There is no guarantee, that this
file ends with newline - it depends on file content and editor used to
create file (some editors append and hide last newline from user while
others do not).
When --status (default) is supplied, additional comment is placed after
template content. If template file ended with newline this results in
additional line being appended (which may be unexpected e.g. when last
line of template is a comment). On the other hand, first line of status
should never be concatenated to last line of template file.
Append newline before status _only_ if template/logfile didn't end with
one already. This way content of template is exactly the way user intended
and there's no chance, that line of status will merge with last line of
template.
There is also interaction with --signoff (which does its own handling
of present or missing newline)...
Remove unnecessary premature cleanup of commit message, which was
implemented for -F, but not for -t.
Is this change distinct from the rest of the patch? If so, it may
deserve its own patch.
Moreover, it lacks justification and explanation of why you consider
the cleanup unnecessary. History [1] indicates that its application to
-F but not -t was intentional.
[1]: bc92377 (commit: fix ending newline for template files, 2015-05-26)
Signed-off-by: Patryk Obara patryk.ob...@gmail.com
---
diff --git a/builtin/commit.c b/builtin/commit.c
index da79ac4..eb41e05 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -666,8 +666,8 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
struct strbuf sb = STRBUF_INIT;
const char *hook_arg1 = NULL;
const char *hook_arg2 = NULL;
- int clean_message_contents = (cleanup_mode != CLEANUP_NONE);
int old_display_comment_prefix;
+ int sb_ends_with_newline = 0;
What does 'sb' mean in sb_ends_with_newline? Is it a reference to
strbuf? If so, it doesn't make the variable name any more meaningful.
/* This checks and barfs if author is badly specified */
determine_author_info(author_ident);
@@ -786,6 +782,9 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
if (auto_comment_line_char)
adjust_comment_line_char(sb);
+
+ sb_ends_with_newline = ends_with(sb.buf, \n);
+
strbuf_release(sb);
/* This checks if committer ident is explicitly given */
@@ -794,6 +793,10 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
int ident_shown = 0;
int saved_color_setting;
struct ident_split ci, ai;
+ int append_newline = (template_file || logfile) ?
!sb_ends_with_newline : 1;
+
+ if (append_newline)
+ fprintf(s-fp, \n);
Did you consider the alternate approach of handling newline processing
immediately upon loading 'logfile' and 'template_file', rather than
delaying processing until this point? Doing it that way would involve
a bit of code repetition but might be easier to reason about since it
would occur before possible interactions in following code (such as
--signoff handling).
if (whence != FROM_COMMIT) {
if (cleanup_mode == CLEANUP_SCISSORS)
@@ -815,7 +818,6 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
: CHERRY_PICK_HEAD));
}
- fprintf(s-fp, \n);
if (cleanup_mode == CLEANUP_ALL)
status_printf(s, GIT_COLOR_NORMAL,
_(Please enter the commit message for your
changes.
--
2.4.1
--
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: [RFC/PATCH v2] create a skeleton for the command git rebase --status
Guillaume Pages guillaume.pa...@ensimag.grenoble-inp.fr writes: Hi, Paul Tan pyoka...@gmail.com : I haven't kept up with the discussion, but I'm wondering: since you need the functionality in wt-status.c, why not implement it in git status? In fact, git-status already shows if there is a rebase in progress, so why not extend it to say which patch/todo line the rebase stopped on? It feels much more natural to me to use git-status to check the status of the rebase, instead of git rebase --status. Thanks, Paul It's a question I have asked myself but since git rebase --status will certainly display more information Historically, I had the idea of git rebase --status and Ensimag students started working on it 2 years ago. The same question came back then during the discussion: why not just in status? This lead to the header in the output of git status (You are currently ..., for rebase/am/bisect/...). But I think there are more relevant information to show (e.g. list of already applied commits, remaining list of commits, possibly truncated if the list is overly long, and information that rebase gave you when stopping like the path to the file being applied). Having them all in git status would make the output really long, for little benefit in day-to-day use. So, to me, it makes sense to have a separate command tell me everything you know about the current state of the rebase. If we choose to use git status, it could be an option to display the full information since it currently gives very few information. I'm not sure what -verbose does but it could be its role. git status --verbose is already taken for show me the diff together with status. (I'm also dreaming of a git status --tutor that would show detailed explanations with pointers to the documentation all for each section of status. Useless for non-beginners, but could be a nice way to teach Git to beginners) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: [RFC/PATCH v2] create a skeleton for the command git rebase --status
Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr writes:
It is an almost empty code sent to validate the global architecture of
this command. It is written in C because git status is already in C
and it seems that it is the current tendency to port shell code to
C. Moreover will likely use code from wt_status to implement this
functionnality. The command calls a helper from a shell script, as it
is made in bisect (bisect--helper.c).
I think this paragraph makes sense in the commit message. My previous
remark was about the wording, not about the relevance of the arguments.
+static const char * const git_rebase_status_helper_usage[] = {
+ N_(git rebase--status--helper),
+ NULL
+};
+
+int cmd_rebase_status__helper(int argc, const char **argv, const char
*prefix)
+{
+ struct option options[] = {
+
+ };
You need to tell parse_options when the array is over (that's C ...) =
OPT_END().
+ argc = parse_options(argc, argv, prefix, options,
+ git_rebase_status_helper_usage, 0);
Actually, you don't need option-parsing at all since you don't pass
arguments to the helper, but why not have this in case we need it later.
No need to resend now, you'll send a new version when you have something
to build on top (or if someone has more important remarks).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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/RFC] send-email: allow multiple emails using --cc --to and --bcc
On Thu, May 28, 2015 at 6:42 AM, Remi Lespinet
remi.lespi...@ensimag.grenoble-inp.fr wrote:
Add the possibility to use a list of emails separated by commas
in flags --cc --to and --bcc instead of having to use one flag
s/--cc --to/--cc, --to/
Ditto in subject.
per email address.
The use-case is to copy-paste a list of addresses from an email.
This change makes it so that we no longer need to cut the list.
The format of email list handled is basic for now:
$ git send-email --to='Foo f...@example.com, b...@example.com'
We thought it would be nice to have a first-step version which works
before handling more complex ones such as names with commas:
$ git send-email --to='Foo, Bar foo...@example.com'
This artificial limitation is imposed by 79ee555b (Check and document
the options to prevent mistakes, 2006-06-21).
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
Contributions-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Helped-by: may be more appropriate than Contributions-by: on this
project. Also, move it above the sign-offs.
---
diff --git a/Documentation/git-send-email.txt
b/Documentation/git-send-email.txt
index 043f345..0aeddcb 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -49,17 +49,21 @@ Composing
of 'sendemail.annotate'. See the CONFIGURATION section for
'sendemail.multiEdit'.
---bcc=address::
+--bcc=[address,...]::
Adding square brackets indicates that the addresses following '=' are
optional, which is incorrect. It should be sufficient merely to add
the comma and ellipsis. Also, the need to quote strings containing
whitespace is a shell issue not specifically related to this option.
That is, people have to understand quoting issues in general, so it
doesn't make sense to mention them literally here. Thus:
--bcc=address,...::
should be sufficient.
Specify a Bcc: value for each email. Default is the value of
'sendemail.bcc'.
-+
-The --bcc option must be repeated for each user you want on the bcc list.
I think it's harmful to remove this line. Although the must is no
longer true following this change, it's still important for people to
know that they can use --bcc multiple times. So, perhaps just reword
it:
This option may be specified multiple times.
+ The format supported for email list is the following:
+ Foo f...@example.com, b...@example.com.
+ Please notice that the email list does not handle commas in
+ email names such as Foo, Bar foo...@example.com.
A few issues:
In order for this to format correctly in Asciidoc, the text needs to
be left-justified (just as was the line which you removed).
The sentence The format supported... seems superfluous. It's merely
repeating what is already clearly indicated by --bcc=address,...,
thus it could easily be dropped without harm.
Mention that commas in names are not currently handled is indeed a
good idea, however, please tends to be an overused and wasted word
in documentation. More tersely:
Note: Addresses containing commas (Foo, Bar ...)
are not currently supported.
The above comments also apply to --cc and --to.
diff --git a/git-send-email.perl b/git-send-email.perl
index ffea500..409ff45 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1052,7 +1038,8 @@ sub sanitize_address {
}
sub sanitize_address_list {
- return (map { sanitize_address($_) } @_);
+ my @addr_list = split_address_list(@_);
+ return (map { sanitize_address($_) } @addr_list);
}
Although, it was convenient from an implementation perspective to plop
the split_address_list() invocation into sanitize_address_list(), it
pollutes sanitize_address_list() by making it do something unrelated
to its purpose.
If you examine places where sanitize_address_list() is called, you will see:
validate_address_list(sanitize_address_list(@xx))
which clearly shows that sanitation and validation are distinct
operations (and each function does only what its name implies). To
continue this idea, the splitting of addresses should be performed
distinctly from the other operations, in this order:
split - sanitize - validate
or:
validate_address_list(sanitize_address_list(
split_address_list(@xx))
That's pretty verbose, so introducing a new function to encapsulates
that behavior might be reasonable.
# Returns the local Fully Qualified Domain Name (FQDN) if available.
@@ -1193,6 +1180,10 @@ sub file_name_is_absolute {
return File::Spec::Functions::file_name_is_absolute($path);
}
+sub split_address_list {
+ return (map { split /\s*,\s*/, $_ } @_);
+}
This is somewhat misnamed. It's not splitting the address list in the
sense that sanitize_address_list() and
Re: [PATCH] glossary: add remote, submodule, superproject
Stefan Beller sbel...@google.com writes: Noticed-by: Philip Oakley philipoak...@iee.org Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Stefan Beller sbel...@google.com --- Documentation/glossary-content.txt | 17 + 1 file changed, 17 insertions(+) The updates in this version relative to the previous one looks very good, at least to me. A bit more comments. diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index bf383c2..23ab692 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -469,6 +469,11 @@ The most notable example is `HEAD`. def_push,push to describe the mapping between remote def_ref,ref and local ref. +[[def_remote]]remote repository:: + A def_repository,repository which is used to track the same + project but resides somewhere else. To communicate with remotes, + see def_fetch,fetch or def_push,push. + The last sentence sounds a tiny bit strange, in that I have to do a bit more than just see the explanation of these commands in order to communicate with remotes. But it probably is just me. @@ -515,6 +520,18 @@ The most notable example is `HEAD`. is created by giving the `--depth` option to linkgit:git-clone[1], and its history can be later deepened with linkgit:git-fetch[1]. +[[def_submodule]]submodule:: + A def_repository,repository that holds the history of a + separate project inside another repository (the latter of + which is called def_superproject, superproject). The + containing superproject knows about the names of (but does + not hold copies of) commit objects of the contained submodules. I agree with one point you mentioned in one of your messages, which is that a submodule is not aware that it is used as part of a larger project. That makes me wonder if the last sentence sits better in the description of the superproject, rather than the description of the submodule. +[[def_superproject]]superproject:: + A def_repository,repository that references other repositories + inside itself as def_submodule,submodules. Perhaps repositories of other projects? Does inside make it clear enough that we are talking about the relationship between working trees of the superproject and submodules? + The superproject + tracks only the remote and the name of the submodule. I am not sure what this sentence means [*1*], and I do not know if (a corrected version of) such a description is necessary here. Thanks. [Footnote] *1* The superproject records a bit more than remote and name in .gitmodules, and of course it records the history of the paths that the submodule is bound to over time, with specific commits from the submodule in its history. -- 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/RFC 2/2] git rebase -i: Warn removed or dupplicated commits
Remi Galan Alfonso remi.galan-alfo...@ensimag.grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: I think there is a difference between (silently) accepting just to be lenient and documenting and advocating mixed case uses. Personally, I'd rather not to see gratuitous flexibility to allow the same thing spelled in 47 different ways for no good reason. It was more of a mistake on our part rather than actually wanting to document mixed case uses. In the v2 of the patch (not sent to the mailing list yet since we want to take into consideration the conclusion of this discussion before) it is entirely in lower case in both the documentation and the code while we silently allow upper and mixed case. Understood; I am not sold on the whole warning business, though. I think I saw you did 'tr [:upper:]' or something like that in the patch; we tend to avoid [:class:] and [=equiv=] when not needed, unless we know that the matching engine used supports them (i.e. it is OK to use them in Perl scripts and it is OK to feed them to the wildmatch-based matcher in Git itself, but not in general shell scripts). As the values can all be represented in US-ASCII, it should be sufficient to do tr 'A-Z' 'a-z', I would think. -- 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
