threads / discuss / 22899

What's cooking in git.git (Mar 2010, #01; Wed, 03)

Subject: What's cooking in git.git (Mar 2010, #01; Wed, 03)

## tl;dr

38 messages between Mar 4, 2010 and Mar 10, 2010.

replies: 37people: 13as markdown or json

Junio C Hamano· Mar 4, 2010, 00:02 UTC · lore

Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them.

-------------------------------------------------- [Graduated to "master"]

* dp/read-not-mmap-small-loose-object (2010-02-21) 1 commit
  (merged to 'next' on 2010-02-21 at fa39a9a)
 + hash-object: don't use mmap() for small files
* np/compress-loose-object-memsave (2010-02-21) 2 commits
  (merged to 'next' on 2010-02-21 at 1e558d6)
 + sha1_file: be paranoid when creating loose objects
 + sha1_file: don't malloc the whole compressed result when writing out objects
* ml/connect-refactor (2010-02-17) 1 commit
  (merged to 'next' on 2010-02-21 at 7361651)
 + connect.c: move duplicated code to a new function 'get_host_and_port'
* ml/encode-header-refactor (2010-02-23) 2 commits
  (merged to 'next' on 2010-02-23 at ac4ec8f)
 + move encode_in_pack_object_header() to a better place
  (merged to 'next' on 2010-02-21 at efe648b)
 + refactor duplicated encode_header in pack-objects and fast-import
* ml/fill-mm-refactor (2010-02-16) 1 commit
  (merged to 'next' on 2010-02-21 at 2fc5570)
 + refactor duplicated fill_mm() in checkout and merge-recursive
* ml/send-pack-transport-refactor (2010-02-16) 1 commit
  (merged to 'next' on 2010-02-21 at db276f4)
 + refactor duplicated code in builtin-send-pack.c and transport.c
* rs/optim-text-wrap (2010-02-19) 4 commits
  (merged to 'next' on 2010-02-21 at 70ef189)
 + utf8.c: speculatively assume utf-8 in strbuf_add_wrapped_text()
 + utf8.c: remove strbuf_write()
 + utf8.c: remove print_spaces()
 + utf8.c: remove print_wrapped_text()
* tr/maint-cherry-pick-list (2010-02-20) 1 commit
  (merged to 'next' on 2010-02-21 at 65fded0)
 + cherry_pick_list: quit early if one side is empty
* ld/maint-diff-quiet-w (2010-02-21) 2 commits
  (merged to 'next' on 2010-02-21 at 4701142)
 + git-diff: add a test for git diff --quiet -w
 + git diff --quiet -w: check and report the status
* jc/for-each-ref (2010-02-13) 4 commits
  (merged to 'next' on 2010-02-21 at c9a6c2f)
 + for-each-ref --format='%(flag)'
 + for-each-ref --format='%(symref) %(symref:short)'
 + builtin-for-each-ref.c: check if we need to peel onion while parsing the format
 + builtin-for-each-ref.c: comment fixes
* jn/gitweb-config-error-die (2010-02-14) 1 commit
  (merged to 'next' on 2010-02-21 at e3ecd65)
 + gitweb: Die if there are parsing errors in config file
* jn/maint-fix-pager (2010-02-22) 8 commits
  (merged to 'next' on 2010-02-23 at 575e0e4)
 + tests: Fix race condition in t7006-pager
  (merged to 'next' on 2010-02-21 at 640e10c)
 + t7006-pager: if stdout is not a terminal, make a new one
 + tests: Add tests for automatic use of pager
 + am: Fix launching of pager
 + git svn: Fix launching of pager
 + git.1: Clarify the behavior of the --paginate option
 + Make 'git var GIT_PAGER' always print the configured pager
 + Fix 'git var' usage synopsis
* ml/color-when (2010-02-16) 1 commit
  (merged to 'next' on 2010-02-21 at d52c051)
 + Add an optional argument for --color options
* hm/imap-send-cram-md5 (2010-02-15) 1 commit
  (merged to 'next' on 2010-02-21 at de8f650)
 + imap-send: support CRAM-MD5 authentication
* gf/maint-sh-setup-nongit-ok (2010-02-16) 1 commit
  (merged to 'next' on 2010-02-21 at aca55e6)
 + require_work_tree broken with NONGIT_OK
* jc/maint-status-preload (2010-02-17) 1 commit
  (merged to 'next' on 2010-02-21 at d79e163)
 + status: preload index to optimize lstat(2) calls
* ac/cvsimport-revision-mapping (2010-02-06) 1 commit
  (merged to 'next' on 2010-02-17 at 6756446)
 + cvsimport: new -R option: generate .git/cvs-revisions mapping
* jn/maint-makedepend (2010-01-26) 5 commits
  (merged to 'next' on 2010-02-21 at 34a3e48)
 + Makefile: drop dependency on $(wildcard */*.h)
 + Makefile: clean up http-walker.o dependency rules
 + Makefile: remove wt-status.h from LIB_H
 + Makefile: make sure test helpers are rebuilt when headers change
 + Makefile: add missing header file dependencies
 (this branch is used by jn/makedepend and jn/master-makedepend.)
* jn/master-makedepend (2010-01-26) 0 commits
 (this branch uses jn/maint-makedepend; is used by jn/makedepend.)
* jn/makedepend (2010-02-28) 10 commits
  (merged to 'next' on 2010-02-28 at 6604fd0)
 + Makefile: clarify definition of TEST_OBJS
  (merged to 'next' on 2010-02-21 at 34a3e48)
 + Makefile: always remove .depend directories on 'make clean'
 + Makefile: tuck away generated makefile fragments in .depend
 + Teach Makefile to check header dependencies
 + Makefile: list standalone program object files in PROGRAM_OBJS
 + Makefile: lazily compute header dependencies
 + Makefile: list generated object files in OBJECTS
 + Makefile: disable default implicit rules
 + Makefile: rearrange dependency rules
 + Makefile: transport.o depends on branch.h now
 (this branch uses jn/maint-makedepend and jn/master-makedepend.)
* jc/grep-author-all-match-implicit (2010-01-17) 1 commit
  (merged to 'next' on 2010-02-17 at 3b7be80)
 + "log --author=me --grep=it" should find intersection, not union
* jh/maint-submodule-status-in-void (2010-03-03) 1 commit
 + submodule summary: Don't barf when invoked in an empty repo

We might also want to enable showing the submodule change, which is currently queued in 'pu'.

-------------------------------------------------- [New Topics]

* bg/apply-fix-blank-at-eof (2010-02-27) 5 commits
 - t3417: Add test cases for "rebase --whitespace=fix"
 - t4124: Add additional tests of --whitespace=fix
 - apply: Allow blank context lines to match beyond EOF
 - apply: Remove the quick rejection test
 - apply: Don't unnecessarily update line lengths in the preimage
Probably ready for 'next'.
* gb/maint-submodule-env (2010-02-25) 5 commits
  (merged to 'next' on 2010-02-25 at 8c22d03)
 + is_submodule_modified(): clear environment properly
 + submodules: ensure clean environment when operating in a submodule
 + shell setup: clear_local_git_env() function
 + rev-parse: --local-env-vars option
 + Refactor list of of repo-local env vars
Ready for 'master'.
* jc/fetch-param (2010-02-24) 3 commits
  (merged to 'next' on 2010-02-25 at e95e252)
 + fetch --all/--multiple: keep all the fetched branch information
 + builtin-fetch --all/--multi: propagate options correctly
 + t5521: fix and modernize
Ready for 'master'.
* jk/maint-push-tracking-wo-remote (2010-02-24) 1 commit
  (merged to 'next' on 2010-02-25 at da946ba)
 + push: fix segfault for odd config
Ready for 'master'.
* mb/shortlog-nongit-stdin (2010-02-24) 1 commit
  (merged to 'next' on 2010-02-25 at d17bb74)
 + shortlog: warn the user when there is no input
Ready for 'master'.
* sg/bash-completion (2010-02-23) 4 commits
 - bash: completion for gitk aliases
 - bash: support user-supplied completion scripts for aliases
 - bash: support user-supplied completion scripts for user's git commands
 - bash: improve aliased command recognition
Perhaps rename _git_frotz -> _git_complete_frotz?  I dunno.
* fn/maint-mkdtemp-compat (2010-02-25) 1 commit
  (merged to 'next' on 2010-02-25 at 2899a47)
 + Fix gitmkdtemp: correct test for mktemp() return value
Ready for 'master'.
* ml/maint-grep-doc (2010-02-25) 4 commits
  (merged to 'next' on 2010-03-02 at a75dfe0)
 + grep docs: document --no-index option
  (merged to 'next' on 2010-02-25 at ec1faf8)
 + grep docs: --cached and <tree>... are incompatible
 + grep docs: use AsciiDoc literals consistently
 + grep docs: pluralize "Example" section
Ready for 'master'.
* fl/askpass (2010-03-03) 2 commits
 - git-core: Support retrieving passwords with GIT_ASKPASS
 - git-svn: Support retrieving passwords with GIT_ASKPASS

As we export GIT_ASKPASS when it is not set but SSH_ASKPASS is from "git" potty (the third patch in the series, which I applied to 'master'), I removed the first hunk that did that by hand.

* as/maint-expire (2010-02-26) 2 commits
  (merged to 'next' on 2010-03-02 at 4015ae4)
 + reflog: honor gc.reflogexpire=never
 + prune: honor --expire=never
* jc/color-attrs (2010-02-27) 1 commit
 - color: allow multiple attributes

Perhaps I should remove the counting, extend COLOR_MAXLEN and remove the test that checks overlong color specification and then merge this to 'next'.

* jc/maint-add-ignored-dir (2010-02-28) 3 commits
 - builtin-add: fix exclude handling
 - tests for "git add ignored-dir/file" without -f
 - t0050: mark non-working test as such
Not quite happy.
* ml/color-grep (2010-02-26) 3 commits
 - grep: Colorize selected, context, and function lines
 - grep: Colorize filename, line number, and separator
 - Add GIT_COLOR_BOLD_* and GIT_COLOR_BG_*
There was a comment about not special casing filename coloring?
* sb/notes-parse-opt (2010-02-27) 1 commit
 - notes: rework subcommands and parse options
 (this branch uses early parts of jh/notes and tr/notes-display.)
* sh/am-keep-cr (2010-02-27) 4 commits
  (merged to 'next' on 2010-03-02 at ffe3c5e)
 + git-am: Add tests for `--keep-cr`, `--no-keep-cr` and `am.keepcr`
 + git-am: Add am.keepcr and --no-keep-cr to override it
 + git-am: Add command line parameter `--keep-cr` passing it to git-mailsplit
 + documentation: 'git-mailsplit --keep-cr' is not hidden anymore
* bw/union-merge-refactor (2010-03-01) 4 commits
 - merge-file: add option to select union merge favor
 - merge-file: add option to specify the marker size
 - refactor merge flags into xmparam_t
 - make union merge an xdl merge favor

The first two are ready for 'next'; the latter two are safe and perhaps useful.

* mg/test-svn-info (2010-03-03) 2 commits
 - t9119-git-svn-info.sh: test with svn 1.6.* as well
 - git-svn: req_svn when needed

-------------------------------------------------- [Stalled]

* sd/format-patch-to (2010-02-17) 1 commit
 - Add 'git format-patch --to=' option and 'format.to' configuration variable.

Shouldn't be too hard to add tests to t4014; other than that looked ready for 'next'.

* sd/log-decorate (2010-02-17) 3 commits
 - log.decorate: usability fixes
 - Add `log.decorate' configuration variable.
 - git_config_maybe_bool()

Probably ready for 'next', except that people need to be warned about having to update their scripts to explicitly pass --no-decorate to keep them working. A good idea to disable this when --pretty was given, just like notes are disabled by default, was floated.

* pb/log-first-parent-p-m (2010-02-10) 1 commit
  (merged to 'next' on 2010-02-17 at 2f8e5ae)
 + git log -p -m: document -m and honor --first-parent

Needs tests but otherwise looked fine. We might want to teach "-m trumps implicit --cc" to "git show", but that is a totally separate topic.

I actually care about this "log -p --first-parent" very much, but if Pasky is counting on that and procrastinating until I write the tests myself, he is in for a disappointment. I don't have that much free time these days. Help is appreciated.

* js/rebase-origin-x (2010-02-05) 1 commit
 - [RFC w/o test and incomplete] rebase: add -x option to record original commit name

I retract my objection against the idea of -x; needs polishing before moving forward.

-------------------------------------------------- [Cooking]

* ld/push-porcelain (2010-02-26) 4 commits
  (merged to 'next' on 2010-03-02 at d15bb1e)
 + git-push: add tests for git push --porcelain
 + git-push: make git push --porcelain print "Done"
 + git-push: send "To <remoteurl>" messages to the standard output in --porcelain mode
 + git-push: fix an advice message so it goes to stderr
* sd/init-template (2010-03-02) 5 commits
  (merged to 'next' on 2010-03-02 at 2d87e3f)
 + wrap-for-bin: do not export an empty GIT_TEMPLATE_DIR
 + t/t0001-init.sh: add test for 'init with init.templatedir set'
 + init: having keywords without value is not a global error.
 + Add a "TEMPLATE DIRECTORY" section to git-init[1].
 + Add `init.templatedir` configuration variable.
* il/loosen-remote-helper-names (2010-02-23) 1 commit
  (merged to 'next' on 2010-02-25 at 5c22a39)
 + Allow '+', '-' and '.' in remote helper names
Ready for 'master'.
* jk/maint-add--interactive-delete (2010-02-22) 1 commit
  (merged to 'next' on 2010-02-24 at 908cef8)
 + add-interactive: fix bogus diff header line ordering
Probably ready for 'master'.
* js/runtime-prefix-trace-not-warn (2010-02-23) 1 commit
  (merged to 'next' on 2010-02-24 at 8d9d305)
 + Print RUNTIME_PREFIX warning only when GIT_TRACE is set
Ready for 'master'.
* lt/deepen-builtin-source (2010-02-22) 1 commit
  (merged to 'next' on 2010-02-25 at 320aa74)
 + Move 'builtin-*' into a 'builtin/' subdirectory

This is a painful one to keep out of 'master' for a long time, as any topic with new builtin commands will need evil merges to adjust to it.

* tc/http-cleanup (2010-03-02) 7 commits
 - remote-curl: init walker only when needed
 - remote-curl: use http_fetch_ref() instead of walker wrapper
 - http: init and cleanup separately from http-walker
 - http-walker: cleanup more thoroughly
 - http-push: remove "|| 1" to enable verbose check
 - t554[01]-http-push: refactor, add non-ff tests
 - t5541-http-push: check that ref is unchanged for non-ff test
Rerolled.
* tr/notes-display (2010-02-23) 11 commits
 - notes: add shorthand --ref to override GIT_NOTES_REF
 - commit --amend: copy notes to the new commit
 - rebase: support automatic notes copying
 - notes: implement helpers needed for note copying during rewrite
 - notes: implement 'git notes copy --stdin'
 - rebase -i: invoke post-rewrite hook
 - rebase: invoke post-rewrite hook
 - commit --amend: invoke post-rewrite hook
 - Documentation: document post-rewrite hook
 - Support showing notes from more than one notes tree
 - test-lib: unset GIT_NOTES_REF to stop it from influencing tests
 (this branch uses early parts of jh/notes; is used by sb/notes-parse-opt.)
Didn't look too carefully except for the second one.
* cw/test-lib-relicense (2010-02-22) 1 commit
 . test-lib.sh: Add explicit license detail, with change from GPLv2 to GPLv2+.
Ack-collection in progress.
* jc/maint-fix-mailinfo-strip (2010-02-19) 1 commit
  (merged to 'next' on 2010-02-24 at 621fa3d)
 + mailinfo: do not strip leading spaces even for a header line
Ready for 'master'.
* ne/pack-local-doc (2010-02-24) 3 commits
  (merged to 'next' on 2010-02-25 at 75cfba5)
 + pack-objects documentation: Fix --honor-pack-keep as well.
 + pack-objects documentation: reword "objects that appear in the standard input"
 + Documentation: pack-objects: Clarify --local's semantics.
Ready for 'master'.
* mm/mkstemps-mode-for-packfiles (2010-02-22) 6 commits
  (merged to 'next' on 2010-02-24 at 31b5903)
 + Use git_mkstemp_mode instead of plain mkstemp to create object files
 + git_mkstemps_mode: don't set errno to EINVAL on exit.
 + Use git_mkstemp_mode and xmkstemp_mode in odb_mkstemp, not chmod later.
 + git_mkstemp_mode, xmkstemp_mode: variants of gitmkstemps with mode argument.
 + Move gitmkstemps to path.c
 + Add a testcase for ACL with restrictive umask.
Ready for 'master'.
* tc/transport-verbosity (2010-02-24) 10 commits
 - transport: update flags to be in running order
 - fetch and pull: learn --progress
 - push: learn --progress
 - transport->progress: use flag authoritatively
 - clone: support multiple levels of verbosity
 - push: support multiple levels of verbosity
 - fetch: refactor verbosity option handling into transport.[ch]
 - Documentation/git-push: put --quiet before --verbose
 - Documentation/git-pull: put verbosity options before merge/fetch ones
 - Documentation/git-clone: mention progress in -v

Didn't look very carefully. Comments from transport people are very much appreciated before moving this forward.

* cp/add-u-pathspec (2010-02-09) 2 commits
  (merged to 'next' on 2010-02-24 at 2f3f2bc)
 + test for add with non-existent pathspec
 + git add -u: die on unmatched pathspec
Not quite happy, but will merge to 'master' shortly anyway.
* nd/root-git (2010-02-14) 5 commits
  (merged to 'next' on 2010-02-25 at bff4955)
 + Add test for using Git at root of file system
 + Support working directory located at root
 + Move offset_1st_component() to path.c
 + init-db, rev-parse --git-dir: do not append redundant slash
 + make_absolute_path(): Do not append redundant slash
Probably ready for 'master'.
* jh/notes (2010-02-24) 32 commits
  (merged to 'next' on 2010-02-24 at c88263d)
 + notes: fix malformed tree entry
 + builtin-notes: Minor (mostly parse_options-related) fixes
  (merged to 'next' on 2010-02-21 at 75fc451)
 + builtin-notes: Add "copy" subcommand for copying notes between objects
 + builtin-notes: Misc. refactoring of argc and exit value handling
 + builtin-notes: Add -c/-C options for reusing notes
 + builtin-notes: Refactor handling of -F option to allow combining -m and -F
 + builtin-notes: Deprecate the -m/-F options for "git notes edit"
 + builtin-notes: Add "append" subcommand for appending to note objects
 + builtin-notes: Add "add" subcommand for adding notes to objects
 + builtin-notes: Add --message/--file aliases for -m/-F options
 + builtin-notes: Add "list" subcommand for listing note objects
 + Documentation: Generalize git-notes docs to 'objects' instead of 'commits'
 + builtin-notes: Add "prune" subcommand for removing notes for missing objects
 + Notes API: prune_notes(): Prune notes that belong to non-existing objects
 + t3305: Verify that removing notes triggers automatic fanout consolidation
 + builtin-notes: Add "remove" subcommand for removing existing notes
 + Teach builtin-notes to remove empty notes
 + Teach notes code to properly preserve non-notes in the notes tree
 + t3305: Verify that adding many notes with git-notes triggers increased fanout
 + t3301: Verify successful annotation of non-commits
 + Builtin-ify git-notes
 + Refactor notes concatenation into a flexible interface for combining notes
 + Notes API: Allow multiple concurrent notes trees with new struct notes_tree
 + Notes API: write_notes_tree(): Store the notes tree in the database
 + Notes API: for_each_note(): Traverse the entire notes tree with a callback
 + Notes API: get_note(): Return the note annotating the given object
 + Notes API: remove_note(): Remove note objects from the notes tree structure
 + Notes API: add_note(): Add note objects to the internal notes tree structure
 + Notes API: init_notes(): Initialize the notes tree from the given notes ref
 + Add tests for checking correct handling of $GIT_NOTES_REF and core.notesRef
 + Notes API: get_commit_notes() -> format_note() + remove the commit restriction
 + Minor cosmetic fixes to notes.c
 (this branch shares commits with sb/notes-parse-opt and tr/notes-display.)
Ready for 'master'.
* cc/reset-keep (2010-01-19) 5 commits
 - reset: disallow using --keep when there are unmerged entries
 - reset: disallow "reset --keep" outside a work tree
 - Documentation: reset: describe new "--keep" option
 - reset: add test cases for "--keep" option
 - reset: add option "--keep" to "git reset"

I am not sure if this series is useful, and even less sure if the usefulness of it outweighs the confusion factor.

Adam Simpkins· Mar 4, 2010, 00:36 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

On Wed, Mar 03, 2010 at 04:02:20PM -0800, Junio C Hamano wrote:
Show 5 quoted lines
> 
> * as/maint-expire (2010-02-26) 2 commits
>   (merged to 'next' on 2010-03-02 at 4015ae4)
>  + reflog: honor gc.reflogexpire=never
>  + prune: honor --expire=never

I take it you didn't like the refactoring of the parsing code? I agree that it is a decent sized change for relatively little added value, although it does seem nice to make the option handling consistent. I was also unsure if the expiration-date handling was common enough that it was worth putting in date.c and config.c, but I didn't really see a better place.

Feel free to let me know if you had any specific complaints about it, or if you think it could be done in another way that so that it would be worth including.

-- 
Adam Simpkins
simpkins@facebook.com
Björn Gustavsson· Mar 4, 2010, 08:26 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

On Thu, Mar 4, 2010 at 1:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
> * bg/apply-fix-blank-at-eof (2010-02-27) 5 commits
>  - t3417: Add test cases for "rebase --whitespace=fix"
>  - t4124: Add additional tests of --whitespace=fix
>  - apply: Allow blank context lines to match beyond EOF
>  - apply: Remove the quick rejection test
>  - apply: Don't unnecessarily update line lengths in the preimage
>
> Probably ready for 'next'.
I have realized that there is one remaining minor issue.

If both --whitespace=fix and --ignore-space-change are given, new blank lines ending in LF may be added to the end of the file. That is the expected behavior if the rest of the file has LF line endings, but not if the rest of the file has CR-LF line endings.

I think I have figured out how to fix it and I will send a patch as soon as I have actually implemented it.

-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB
Junio C Hamano· Mar 4, 2010, 18:26 UTC · re: Björn Gustavsson · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

Björn Gustavsson <bgustavsson@gmail.com> writes:
Show 8 quoted lines
> If both --whitespace=fix and --ignore-space-change are given,
> new blank lines ending in LF may be added to the end of the file.
> That is the expected behavior if the rest of the file has
> LF line endings, but not if the rest of the file has CR-LF
> line endings.
>
> I think I have figured out how to fix it and I will send
> a patch as soon as I have actually implemented it.
Thanks.
Tay Ray Chuan· Mar 4, 2010, 12:09 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

Hi,
On Thu, Mar 4, 2010 at 8:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> * tc/transport-verbosity (2010-02-24) 10 commits
>  - transport: update flags to be in running order
>  - fetch and pull: learn --progress
>  - push: learn --progress
>  - transport->progress: use flag authoritatively
>  - clone: support multiple levels of verbosity
>  - push: support multiple levels of verbosity
>  - fetch: refactor verbosity option handling into transport.[ch]
>  - Documentation/git-push: put --quiet before --verbose
>  - Documentation/git-pull: put verbosity options before merge/fetch ones
>  - Documentation/git-clone: mention progress in -v
>
> Didn't look very carefully.  Comments from transport people are very much
> appreciated before moving this forward.

the last discussion on this topic centred around the git-pull documentation, which this latest iteration has addressed. Jeff also gave his OK for the previous iteration.

I also noticed a merge conflict - I think the resolution looked ok.
-- 
Cheers,
Ray Chuan
Junio C Hamano· Mar 4, 2010, 18:26 UTC · re: Tay Ray Chuan · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

Tay Ray Chuan <rctay89@gmail.com> writes:
Show 8 quoted lines
> On Thu, Mar 4, 2010 at 8:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> * tc/transport-verbosity (2010-02-24) 10 commits
> ...
> the last discussion on this topic centred around the git-pull
> documentation, which this latest iteration has addressed. Jeff also
> gave his OK for the previous iteration.
>
> I also noticed a merge conflict - I think the resolution looked ok.
Thanks.
Junio C Hamano· Mar 4, 2010, 21:42 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

Junio C Hamano <gitster@pobox.com> writes:
Show 9 quoted lines
> * cc/reset-keep (2010-01-19) 5 commits
>  - reset: disallow using --keep when there are unmerged entries
>  - reset: disallow "reset --keep" outside a work tree
>  - Documentation: reset: describe new "--keep" option
>  - reset: add test cases for "--keep" option
>  - reset: add option "--keep" to "git reset"
>
> I am not sure if this series is useful, and even less sure if the
> usefulness of it outweighs the confusion factor.

Regarding this, I've been thinking about how I would explain this new feature to end users (both new ones and old timers) as a good addition. I still haven't reached a satisfactory explanation, but here is my "WIP" try to describe a scenario.

I understand that in essense, "reset --keep" does exactly a "checkout <commit>" does but without detaching the HEAD. I often deliberately stay on detached HEAD because I find it highly useful that I can jump around freely with "checkout <commit>" once the head is detached (I would stay in a detached HEAD state and keep local changes around). I think there must be a similar usefulness I can gain by "reset --keep".

But I am not really succeeding to explain that to potential users.
    I have built commits A, B and C on my 'topic' branch, and the contents
    in the working tree is a checkout of C (which is at the tip of the
    branch).  I can make a small improvement, and start hacking.  And then
    I realize that the change I just did, which I haven't committed nor
    even added, is an improvement to commit A.
    I could do:
        $ git checkout topic~2              ;# to detach at A
        $ git commit --amend -a             ;# to improve on A
        $ git rebase --onto HEAD @{1} topic ;# rebase the rest and come back
    to fix up A and rebuild B and C on top of it.  With "reset --keep",
    I could do this instead:
        $ last=$(git rev-parse HEAD)
        $ git reset --keep topic~2
        $ git commit --amend -a
        $ git rebase --onto HEAD @{1} @{2}  ;# rebase the reset
        $ git branch -f topic               ;# and come
        $ git checkout topic                ;# back
The above however is clearly not an improvement.

So far, the _only_ use case I can think of that "reset --keep" may be superiour than anything existing is this:

    I have built commits A, B and C on my 'topic' branch, and the contents
    in the working tree is a checkout of C (which is at the tip of the
    branch).  I can make a small improvement, and start hacking.  And then
    I realize that the change I just did, which I haven't committed nor
    even added, is an improvement to commit A.  Also I realize that B and
    C are completely bogus, and I want to get rid of them.
    I could do:
        $ git checkout topic~2		;# to detach at A
        $ git commit --amend -a         ;# fix it
        $ git branch -f topic           ;# the rest I do not need
        $ git checkout topic	        ;# and now on the branch
    but it would be far easier if I can do this:
        $ git reset --keep topic~2
        $ git commit --amend -a

You have some addition in Documentation/git-reset.txt in this topic, and the last example (starting at around line 350) may be describing this situation, but it was not very clear to me.

    Keep changes in working tree while discarding some previous commits::
    Suppose you are working on something and you commit it, and then you
    continue working a bit more, but now you think that what you have in
    your working tree should be in another branch that has nothing to do
    with what you commited previously. You can start a new branch and
    reset it while keeping the changes in your work tree.
    ------------
    $ git tag start
    $ git branch branch1
I take it that this is supposed to be "checkout -b branch1".
    $ edit
    $ git commit ...                            <1>
    $ edit
    $ git branch branch2                        <2>
I take it that this is supposed to be "checkout -b branch2".
    $ git reset --keep start                    <3>
    ------------
    <1> This commits your first edits in branch1.
    <2> This creates branch2, but unfortunately it contains the previous
    commit that you don't want in this branch.
    <3> This removes the unwanted previous commit, but this keeps the
    changes in your working tree.

The above sequence is not very convincing. After you edited the second time, you create branch2 and that is presumably because you realized that the change in the work tree belongs to a separate topic. It would be a lot more natural to do this:

    $ git tag start ;# we do not have to tag, but just to make the
                       remainder of the illustration easier to read...
    $ git checkout -b branch1
    $ edit	    ;# do the work for the first topic
    $ git commit    ;# and commit
    $ edit          ;# start working more and then realize that the
    		       change belongs to a separate topic, and the previous
                       commit is unrelated to that new topic
    $ git checkout -b branch2 start 
    $ edit          ;# continue working
    $ git commit    ;# and conclude it
so the example makes the use of "reset --keep" look artificial.
Junio C Hamano· Mar 5, 2010, 00:49 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

Junio C Hamano <gitster@pobox.com> writes:
Show 31 quoted lines
>     $ git branch branch2                        <2>
>
> I take it that this is supposed to be "checkout -b branch2".
>
>     $ git reset --keep start                    <3>
>     ------------
>
>     <1> This commits your first edits in branch1.
>     <2> This creates branch2, but unfortunately it contains the previous
>     commit that you don't want in this branch.
>     <3> This removes the unwanted previous commit, but this keeps the
>     changes in your working tree.
>
> The above sequence is not very convincing.  After you edited the second
> time, you create branch2 and that is presumably because you realized that
> the change in the work tree belongs to a separate topic.  It would be a
> lot more natural to do this:
>
>     $ git tag start ;# we do not have to tag, but just to make the
>                        remainder of the illustration easier to read...
>     $ git checkout -b branch1
>     $ edit	    ;# do the work for the first topic
>     $ git commit    ;# and commit
>     $ edit          ;# start working more and then realize that the
>     		       change belongs to a separate topic, and the previous
>                        commit is unrelated to that new topic
>     $ git checkout -b branch2 start 
>     $ edit          ;# continue working
>     $ git commit    ;# and conclude it
>
> so the example makes the use of "reset --keep" look artificial.

Nah, what was I thinking. If I rephrase your side note <2> and <3> a little bit, everything makes sense. Perhaps like so:

    <2> In the ideal world, you could have realized that the earlier
    commit did not belong to the new topic when you created and switched
    to branch2 (i.e. "git checkout -b branch2 start"), but nobody is
    perfect.
    <3> But you can use "reset --keep" to remove the unwanted commit after
    you switched to "branch2".

And it becomes very clear that "reset --keep" is a sensible way to recover from this mistake. No need to do "read-tree -m -u" followed by "reset" anymore.

Do you think I finally understood what "reset --keep" is about?
Jonathan Nieder· Mar 5, 2010, 16:25 UTC · re: Junio C Hamano · lore

git reset --keep (Re: What's cooking in git.git (Mar 2010, #01; Wed, 03))

Junio C Hamano wrote:
> Do you think I finally understood what "reset --keep" is about?

Probably. :) Let me take the opportunity to give some examples of what I am hoping to use it for, to see if I am crazy.

Show 14 quoted lines
> Nah, what was I thinking.  If I rephrase your side note <2> and <3> a
> little bit, everything makes sense.  Perhaps like so:
> 
>     <2> In the ideal world, you could have realized that the earlier
>     commit did not belong to the new topic when you created and switched
>     to branch2 (i.e. "git checkout -b branch2 start"), but nobody is
>     perfect.
> 
>     <3> But you can use "reset --keep" to remove the unwanted commit after
>     you switched to "branch2".
> 
> And it becomes very clear that "reset --keep" is a sensible way to recover
> from this mistake.  No need to do "read-tree -m -u" followed by "reset"
> anymore.

Yes, this (recovery from a wrong choice of starting commit for a new branch) makes sense. Here are some other planned uses:

1. Helping people new to git.

A person not very familiar with git comes to me asking how to undo the last couple of commits. After a quick conversation, it becomes clear that the commits in question were not pushed out to any public repository and that this person does not feel it would be useful to publish the problem commits.

Currently, I would have to advise such a person to use
	git reset --hard HEAD^^
I would prefer to recommend
	git reset --keep HEAD^^

because if there are uncommitted changes then it will give a "needs update" message (right?) and I can help the person to deal with it.

2. Splitting up a huge patch.

Suppose I have a huge patch consisting of several unrelated changes applied to the work tree but not commited. I want to split it into logical changes, commiting each one, and when I am done I will use a loop reading from git rev-list to test all the resulting commits automatically. A workflow for this looks something like the following:

 git checkout -b series1
 git add -p
 git commit
 git add -p
 git commit
 git checkout -b series2 appropriate-base
 git add -p
 git commit
 ...
Having 'git reset --keep' available would add some flexibility:
 * As you mentioned, reset --keep would let me recover from 'git
   checkout -b' to the wrong commit.
 * As in example 1, if some part of the patch turns out to be a
   bad idea after all, I can try to discard it.

A 'git stash' might be worth avoiding in some cases because it touches unrelated files, which means wasted time rebuilding everything.

3. Keeping unrelated extra changes around.

Suppose I am Linus, and I keep on forgetting to update the version number in a file named version.h or something. So I update it in advance as soon as I remember, but I do not commit the change or register it in the index because it is not time yet.

Then in almost every instance when I would have normally used 'reset --hard', I should use 'reset --keep' instead. The only exception is when I am mean “screw it all, reset to a completely known state”; in that case, I will have to update version.h by hand again.

Christian Couder· Mar 5, 2010, 21:08 UTC · re: Jonathan Nieder · lore

Re: git reset --keep (Re: What's cooking in git.git (Mar 2010, #01; Wed, 03))

On Friday 05 March 2010 17:25:21 Jonathan Nieder wrote:
Show 19 quoted lines
>
> 1. Helping people new to git.
> 
> A person not very familiar with git comes to me asking how to undo
> the last couple of commits.  After a quick conversation, it becomes
> clear that the commits in question were not pushed out to any public
> repository and that this person does not feel it would be useful to
> publish the problem commits.
> 
> Currently, I would have to advise such a person to use
> 
> 	git reset --hard HEAD^^
> 
> I would prefer to recommend
> 
> 	git reset --keep HEAD^^
> 
> because if there are uncommitted changes then it will give a "needs
> update" message (right?) and I can help the person to deal with it.

If the uncommited changes are in files that are not touched by the discarded commits then it will silently work and will keep your uncommited changes. If the uncommited changes are in files touched by the discarded commits then it will fail with an error message like this:

error: Entry 'foo' not uptodate. Cannot merge. fatal: Could not reset index file to revision 'HEAD^^'.

Best regards, Christian.

Christian Couder· Mar 5, 2010, 17:32 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

On Friday 05 March 2010 01:49:21 Junio C Hamano wrote:
Show 49 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> >     $ git branch branch2                        <2>
> >
> > I take it that this is supposed to be "checkout -b branch2".
> >
> >     $ git reset --keep start                    <3>
> >     ------------
> >
> >     <1> This commits your first edits in branch1.
> >     <2> This creates branch2, but unfortunately it contains the previous
> >     commit that you don't want in this branch.
> >     <3> This removes the unwanted previous commit, but this keeps the
> >     changes in your working tree.
> >
> > The above sequence is not very convincing.  After you edited the second
> > time, you create branch2 and that is presumably because you realized that
> > the change in the work tree belongs to a separate topic.  It would be a
> > lot more natural to do this:
> >
> >     $ git tag start ;# we do not have to tag, but just to make the
> >                        remainder of the illustration easier to read...
> >     $ git checkout -b branch1
> >     $ edit	    ;# do the work for the first topic
> >     $ git commit    ;# and commit
> >     $ edit          ;# start working more and then realize that the
> >     		       change belongs to a separate topic, and the previous
> >                        commit is unrelated to that new topic
> >     $ git checkout -b branch2 start
> >     $ edit          ;# continue working
> >     $ git commit    ;# and conclude it
> >
> > so the example makes the use of "reset --keep" look artificial.
> 
> Nah, what was I thinking.  If I rephrase your side note <2> and <3> a
> little bit, everything makes sense.  Perhaps like so:
> 
>     <2> In the ideal world, you could have realized that the earlier
>     commit did not belong to the new topic when you created and switched
>     to branch2 (i.e. "git checkout -b branch2 start"), but nobody is
>     perfect.
> 
>     <3> But you can use "reset --keep" to remove the unwanted commit after
>     you switched to "branch2".
> 
> And it becomes very clear that "reset --keep" is a sensible way to recover
> from this mistake.  No need to do "read-tree -m -u" followed by "reset"
> anymore.
> 
> Do you think I finally understood what "reset --keep" is about?
Yes I think so. Thanks for that.

I will rework the documentation patch according to your remarks and perhaps Jonathan Nieder's remarks too.

Thanks both, Christian.

Thomas Rast· Mar 4, 2010, 22:21 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

On Thursday 04 March 2010 01:02:20 Junio C Hamano wrote:
Show 16 quoted lines
> 
> * tr/notes-display (2010-02-23) 11 commits
>  - notes: add shorthand --ref to override GIT_NOTES_REF
>  - commit --amend: copy notes to the new commit
>  - rebase: support automatic notes copying
>  - notes: implement helpers needed for note copying during rewrite
>  - notes: implement 'git notes copy --stdin'
>  - rebase -i: invoke post-rewrite hook
>  - rebase: invoke post-rewrite hook
>  - commit --amend: invoke post-rewrite hook
>  - Documentation: document post-rewrite hook
>  - Support showing notes from more than one notes tree
>  - test-lib: unset GIT_NOTES_REF to stop it from influencing tests
>  (this branch uses early parts of jh/notes; is used by sb/notes-parse-opt.)
> 
> Didn't look too carefully except for the second one.

I hope to find the time to make a reroll this weekend. Thanks for your patience...

Meanwhile, I revived the automatic gmane-and-then-some notes generation bot I wrote a year ago: point your core.notesref (or notes.displayref with the above series) at

  git://repo.or.cz/git/trast.git notes/full
or for a version that only supplies Message-ID and gmane permalink,
  git://repo.or.cz/git/trast.git notes/terse
-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Mark Lodato· Mar 5, 2010, 01:30 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

On Wed, Mar 3, 2010 at 7:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 6 quoted lines
> * ml/color-grep (2010-02-26) 3 commits
>  - grep: Colorize selected, context, and function lines
>  - grep: Colorize filename, line number, and separator
>  - Add GIT_COLOR_BOLD_* and GIT_COLOR_BG_*
>
> There was a comment about not special casing filename coloring?

The disagreement is whether --name-only output should be colored or not. In the patch, it is not, which I argue makes more sense. When --name-only is given, the only thing output is filenames. Having them all be the same color adds no information, and I personally find it annoying to see one big block of the same color. GNU grep does color the filenames with --name-only. Michael Witten argues that this makes the output consistent: whenever it's a filename, it's colored. [1] He also thinks that matching GNU grep's behavior is important. He didn't convince me and I didn't convince him, so it would be nice to have more opinions on this.

Mark Lodato· Mar 5, 2010, 01:32 UTC · re: Mark Lodato · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

On Thu, Mar 4, 2010 at 8:30 PM, Mark Lodato <lodatom@gmail.com> wrote:
Show 18 quoted lines
> On Wed, Mar 3, 2010 at 7:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> * ml/color-grep (2010-02-26) 3 commits
>>  - grep: Colorize selected, context, and function lines
>>  - grep: Colorize filename, line number, and separator
>>  - Add GIT_COLOR_BOLD_* and GIT_COLOR_BG_*
>>
>> There was a comment about not special casing filename coloring?
>
> The disagreement is whether --name-only output should be colored or
> not.  In the patch, it is not, which I argue makes more sense.  When
> --name-only is given, the only thing output is filenames.  Having them
> all be the same color adds no information, and I personally find it
> annoying to see one big block of the same color. GNU grep does color
> the filenames with --name-only.  Michael Witten argues that this makes
> the output consistent: whenever it's a filename, it's colored. [1]  He
> also thinks that matching GNU grep's behavior is important.  He didn't
> convince me and I didn't convince him, so it would be nice to have
> more opinions on this.
Sorry, forgot the footnote:

[1] Except that GNU grep does not color the filename is "Binary file <file> matches." This patch does color it.

Junio C Hamano· Mar 5, 2010, 03:23 UTC · re: Mark Lodato · lore

Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

Mark Lodato <lodatom@gmail.com> writes:
Show 10 quoted lines
> The disagreement is whether --name-only output should be colored or
> not.  In the patch, it is not, which I argue makes more sense.  When
> --name-only is given, the only thing output is filenames.  Having them
> all be the same color adds no information, and I personally find it
> annoying to see one big block of the same color. GNU grep does color
> the filenames with --name-only.  Michael Witten argues that this makes
> the output consistent: whenever it's a filename, it's colored. [1]  He
> also thinks that matching GNU grep's behavior is important.  He didn't
> convince me and I didn't convince him, so it would be nice to have
> more opinions on this.

I don't have a very strong preference, but I would say painting filenames in --name-only output the same way would make more sense than not doing so, as it is obviously consistent if we paint the name of the file exactly the same way whenever we write it at the leftmost column as the hit label, no matter what options are in effect, e.g. -c, -l, or nothing.

As to the coloring of <foo> in "Binary file <foo> matches", I don't think it matters very much which way you choose. That string is an oddball to begin with---it isn't even prefixed with the filename like normal "hit" is:

    $ git grep Q t/test4*.png t/Makefile
    Binary file t/test4012.png matches
    t/Makefile:SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
    t/Makefile:     @echo "*** $@ ***"; GIT_CONFIG=.git/config ...
    t/Makefile:     '$(SHELL_PATH_SQ)' ./aggregate-results.sh test-results/t*-*    

and I think it is deliberately made an oddball, i.e. it shouldn't be like this:

    $ git grep Q t/test4*.png t/Makefile
    t/test4012.png: Binary file t/test4012.png matches
    t/Makefile:SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
    t/Makefile:     @echo "*** $@ ***"; GIT_CONFIG=.git/config ...
    t/Makefile:     '$(SHELL_PATH_SQ)' ./aggregate-results.sh test-results/t*-*    

because if you did so, you cannot tell if t/test4012.png is a binary file, or it has that matched string anymore (well you can---the string doesn't have Q, but I think you know what I mean).

That makes me think that it is not even violating consistency if we treat the <foo> in "Binary file <foo> matches" differently from the usual filename label at the leftmost column. We do not have to be consistent there, as the whole point of the line being an oddball is because it fundamentally wants to be shown differently.

On the other hand, painting <foo> in the same "filename" color may make it easier to spot for color-loving people.

IOW, you can argue both ways, and both argument equally makes sense. That is why I don't think it matters very much.

And in such a case, it is typically safer to follow existing practices if there are any. If GNU paints it, we should. If GNU doesn't, we probably shouldn't.

Miklos Vajna· Mar 6, 2010, 00:39 UTC · re: Junio C Hamano · lore

[PATCH] Add tests for git format-patch --to and format.to config option

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Wed, Mar 03, 2010 at 04:02:20PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
Show 5 quoted lines
> * sd/format-patch-to (2010-02-17) 1 commit
>  - Add 'git format-patch --to=' option and 'format.to' configuration variable.
>
> Shouldn't be too hard to add tests to t4014; other than that looked ready
> for 'next'.
Here is a patch that does so.
 t/t4014-format-patch.sh |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index f2a2aaa..9305c98 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -531,6 +531,26 @@ test_expect_success 'format-patch --in-reply-to' '
 	grep "^References: <baz@foo.bar>" patch8
 '
 
+test_expect_success 'command line to' '
+
+	git format-patch --to="R. E. Cipient <rcipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch9 &&
+	grep "^To: R. E. Cipient <rcipient@example.com>" patch9
+'
+
+test_expect_success 'configuration to' '
+
+	git config --replace-all format.to "R. E. Cipient <rcipient@example.com>" &&
+	git format-patch --stdout master..side | sed -e "/^\$/q" >patch10 &&
+	grep "^To: R. E. Cipient <rcipient@example.com>" patch10
+'
+
+test_expect_success 'additional command line to' '
+
+	git format-patch --to="S. E. Cipient <scipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch11 &&
+	grep "^To: R. E. Cipient <rcipient@example.com>,\$" patch11 &&
+	grep "^ *S. E. Cipient <scipient@example.com>\$" patch11
+'
+
 test_expect_success 'format-patch --signoff' '
 	git format-patch -1 --signoff --stdout |
 	grep "^Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
-- 
1.7.0
Junio C Hamano· Mar 6, 2010, 02:21 UTC · re: Miklos Vajna · lore

Re: [PATCH] Add tests for git format-patch --to and format.to config option

Miklos Vajna <vmiklos@frugalware.org> writes:
Show 6 quoted lines
> +test_expect_success 'additional command line to' '
> +
> +	git format-patch --to="S. E. Cipient <scipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch11 &&
> +	grep "^To: R. E. Cipient <rcipient@example.com>,\$" patch11 &&
> +	grep "^ *S. E. Cipient <scipient@example.com>\$" patch11
> +'

This reveals that --to does not follow the usual rule to override corresponding configuration. Is that really what we want? IOW, when the command line says scipient, shouldn't we stop sending to recipient that comes from the configuration? How else would a user override this?

So I guess the topic wasn't ready for 'next' yet, after all.
Thanks.
Miklos Vajna· Mar 6, 2010, 21:06 UTC · re: Junio C Hamano · lore

[PATCH] format-patch --to: overwrite format.to contents, don't append it

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Fri, Mar 05, 2010 at 06:21:42PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> This reveals that --to does not follow the usual rule to override
> corresponding configuration.  Is that really what we want?  IOW, when the
> command line says scipient, shouldn't we stop sending to recipient that
> comes from the configuration?  How else would a user override this?

Fair enough, here is a patch to update both the testcase and the code to the wished behaviour.

 builtin-log.c           |    6 ++++++
 t/t4014-format-patch.sh |    5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 5d23a67..cc28357 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -465,6 +465,7 @@ static int extra_hdr_alloc;
 static char **extra_to;
 static int extra_to_nr;
 static int extra_to_alloc;
+static int extra_to_config = 0;
 
 static char **extra_cc;
 static int extra_cc_nr;
@@ -507,6 +508,7 @@ static int git_format_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "format.to")) {
 		if (!value)
 			return config_error_nonbool(var);
+		extra_to_config = 1;
 		ALLOC_GROW(extra_to, extra_to_nr + 1, extra_to_alloc);
 		extra_to[extra_to_nr++] = xstrdup(value);
 		return 0;
@@ -884,6 +886,10 @@ static int header_callback(const struct option *opt, const char *arg, int unset)
 
 static int to_callback(const struct option *opt, const char *arg, int unset)
 {
+	if (extra_to_config) {
+		extra_to_config = 0;
+		extra_to_nr = 0;
+	}
 	ALLOC_GROW(extra_to, extra_to_nr + 1, extra_to_alloc);
 	extra_to[extra_to_nr++] = xstrdup(arg);
 	return 0;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 9305c98..6fc071a 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -546,9 +546,10 @@ test_expect_success 'configuration to' '
 
 test_expect_success 'additional command line to' '
 
+	git config --unset-all format.headers &&
 	git format-patch --to="S. E. Cipient <scipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch11 &&
-	grep "^To: R. E. Cipient <rcipient@example.com>,\$" patch11 &&
-	grep "^ *S. E. Cipient <scipient@example.com>\$" patch11
+	! grep "R. E. Cipient <rcipient@example.com>" patch11 &&
+	grep "^To: S. E. Cipient <scipient@example.com>\$" patch11
 '
 
 test_expect_success 'format-patch --signoff' '
-- 
1.7.0
Stephen Boyd· Mar 7, 2010, 00:06 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add tests for git format-patch --to and format.to config option

On 03/05/2010 06:21 PM, Junio C Hamano wrote:
Show 7 quoted lines
> This reveals that --to does not follow the usual rule to override
> corresponding configuration.  Is that really what we want?  IOW, when the
> command line says scipient, shouldn't we stop sending to recipient that
> comes from the configuration?  How else would a user override this?
>
> So I guess the topic wasn't ready for 'next' yet, after all.
>   

The same applies to the fomat.headers and format.cc config options. How is this different?

Miklos Vajna· Mar 7, 2010, 01:20 UTC · re: Stephen Boyd · lore

Re: [PATCH] Add tests for git format-patch --to and format.to config option

On Sat, Mar 06, 2010 at 04:06:18PM -0800, Stephen Boyd <bebarino@gmail.com> wrote:
> The same applies to the fomat.headers and format.cc config options. How
> is this different?

I think having multiple Cc: and custom headers is a regular use case, while having multiple To: headers is rare.

Junio C Hamano· Mar 7, 2010, 03:42 UTC · re: Stephen Boyd · lore

Re: [PATCH] Add tests for git format-patch --to and format.to config option

Stephen Boyd <bebarino@gmail.com> writes:
Show 10 quoted lines
> On 03/05/2010 06:21 PM, Junio C Hamano wrote:
>> This reveals that --to does not follow the usual rule to override
>> corresponding configuration.  Is that really what we want?  IOW, when the
>> command line says scipient, shouldn't we stop sending to recipient that
>> comes from the configuration?  How else would a user override this?
>>
>> So I guess the topic wasn't ready for 'next' yet, after all.
>
> The same applies to the fomat.headers and format.cc config options. How
> is this different?

Not different. Perhaps we should fix them now you noticed they share the same problem?

An obvious alternative is to keep format.to not get overriden by --to as the original patch did; that would at least make the handling between config and option consistent inside the command, but at the same time, it means format-patch behaves differently from everything else in git.

I don't have strong preference either way myself, and while I know "fixing" the ones you listed would affect existing users, I have this suspicion that it wouldn't be too big a problem. After all, format-patch does not _send_ mails; these MUA-like "features" doesn't belong to the program, and nobody should be relying on them heavily in the first place.

Stephen Boyd· Mar 7, 2010, 09:43 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add tests for git format-patch --to and format.to config option

On 03/06/2010 07:42 PM, Junio C Hamano wrote:
Show 15 quoted lines
> Stephen Boyd <bebarino@gmail.com> writes:
>
>   
>> The same applies to the fomat.headers and format.cc config options. How
>> is this different?
>>     
> Not different.  Perhaps we should fix them now you noticed they share the
> same problem?
>
>
> An obvious alternative is to keep format.to not get overriden by --to as
> the original patch did; that would at least make the handling between
> config and option consistent inside the command, but at the same time, it
> means format-patch behaves differently from everything else in git.
>   

Actually, I think the same applies to send-email too? There's sendemail.to and sendemail.cc which can't be overridden. At least the email associated commands are all quirky ;-)

Honestly though, I think you're right about fixing them. We have the option of making them consistent with the rest of git with a little bit of work. If you say --no-cc or --no-add-headers or --no-to the respective config should be overriden. If you say --to or --cc or --add-headers it should be appended. I doubt anyone would find that surprising since --no-* doesn't do anything right now.

Junio C Hamano· Mar 7, 2010, 18:38 UTC · re: Stephen Boyd · lore

Re: [PATCH] Add tests for git format-patch --to and format.to config option

Stephen Boyd <bebarino@gmail.com> writes:
Show 6 quoted lines
> ... We have the
> option of making them consistent with the rest of git with a little bit
> of work. If you say --no-cc or --no-add-headers or --no-to the
> respective config should be overriden. If you say --to or --cc or
> --add-headers it should be appended. I doubt anyone would find that
> surprising since --no-* doesn't do anything right now.

That sounds like a sensible and practical way out, as it won't break existing setup that expects the additive behaviour these two command somehow ended up with, while allowing --no-* to override the config when necessary.

Stephen Boyd· Mar 7, 2010, 21:33 UTC · re: Junio C Hamano · lore

[PATCH 0/4] format-patch and send-email ignoring config settings

Ok this does the --no-* thing to allow the user to say "I want to ignore the config settings this time". I couldn't think of a better way to implement appending and overriding without having to say --no-to and --to in the same command line invocation. Seems to work ok though.

My perl is pretty bad so please check send-email. 
You get an ancient bugfix for free too. Enjoy.
Stephen Boyd (4):
  send-email: actually add bcc headers
  format-patch: use a string_list for headers
  format-patch: add --no-cc, --no-to, and --no-add-headers
  send-email: add --no-cc, --no-to, and --no-bcc
 builtin-log.c           |   91 ++++++++++++++++++++++++----------------------
 git-send-email.perl     |   19 ++++++++--
 t/t4014-format-patch.sh |   38 +++++++++++++++++++
 t/t9001-send-email.sh   |   39 ++++++++++++++++++++
 4 files changed, 139 insertions(+), 48 deletions(-)
Stephen Boyd· Mar 7, 2010, 22:46 UTC · re: Stephen Boyd · lore

[PATCHv2 0/3] format-patch and send-email ignoring config settings

Changes since v1:
 * No more initializers 
 * Better tests for send-email
 * Dropped first patch
Stephen Boyd (3):
  format-patch: use a string_list for headers
  format-patch: add --no-cc, --no-to, and --no-add-headers
  send-email: add --no-cc, --no-to, and --no-bcc
 builtin-log.c           |   94 +++++++++++++++++++++++++----------------------
 git-send-email.perl     |   14 +++++--
 t/t4014-format-patch.sh |   38 +++++++++++++++++++
 t/t9001-send-email.sh   |   66 +++++++++++++++++++++++++++++++++
 4 files changed, 164 insertions(+), 48 deletions(-)
Stephen Boyd· Mar 7, 2010, 22:46 UTC · re: Stephen Boyd · lore

[PATCHv2 1/3] format-patch: use a string_list for headers

In the next patch we'll need to clear the header lists if the user specifies --no-add-headers or --no-to or --no-cc. This actually cuts down on the code a bit too.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-log.c |   73 ++++++++++++++++++++++++++------------------------------
 1 files changed, 34 insertions(+), 39 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 5d23a67..ca241af 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -458,35 +458,28 @@ static int auto_number = 1;
 
 static char *default_attach = NULL;
 
-static char **extra_hdr;
-static int extra_hdr_nr;
-static int extra_hdr_alloc;
-
-static char **extra_to;
-static int extra_to_nr;
-static int extra_to_alloc;
-
-static char **extra_cc;
-static int extra_cc_nr;
-static int extra_cc_alloc;
+static struct string_list extra_hdr;
+static struct string_list extra_to;
+static struct string_list extra_cc;
 
 static void add_header(const char *value)
 {
+	struct string_list_item *i;
 	int len = strlen(value);
 	while (len && value[len - 1] == '\n')
 		len--;
+
 	if (!strncasecmp(value, "to: ", 4)) {
-		ALLOC_GROW(extra_to, extra_to_nr + 1, extra_to_alloc);
-		extra_to[extra_to_nr++] = xstrndup(value + 4, len - 4);
-		return;
+		i = string_list_append(value + 4, &extra_to);
+		len -= 4;
+	} else if (!strncasecmp(value, "cc: ", 4)) {
+		i = string_list_append(value + 4, &extra_cc);
+		len -= 4;
+	} else {
+		i =string_list_append(value, &extra_hdr);
 	}
-	if (!strncasecmp(value, "cc: ", 4)) {
-		ALLOC_GROW(extra_cc, extra_cc_nr + 1, extra_cc_alloc);
-		extra_cc[extra_cc_nr++] = xstrndup(value + 4, len - 4);
-		return;
-	}
-	ALLOC_GROW(extra_hdr, extra_hdr_nr + 1, extra_hdr_alloc);
-	extra_hdr[extra_hdr_nr++] = xstrndup(value, len);
+
+	i->string[len] = '\0';
 }
 
 #define THREAD_SHALLOW 1
@@ -507,15 +500,13 @@ static int git_format_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "format.to")) {
 		if (!value)
 			return config_error_nonbool(var);
-		ALLOC_GROW(extra_to, extra_to_nr + 1, extra_to_alloc);
-		extra_to[extra_to_nr++] = xstrdup(value);
+		string_list_append(value, &extra_to);
 		return 0;
 	}
 	if (!strcmp(var, "format.cc")) {
 		if (!value)
 			return config_error_nonbool(var);
-		ALLOC_GROW(extra_cc, extra_cc_nr + 1, extra_cc_alloc);
-		extra_cc[extra_cc_nr++] = xstrdup(value);
+		string_list_append(value, &extra_cc);
 		return 0;
 	}
 	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
@@ -884,15 +875,13 @@ static int header_callback(const struct option *opt, const char *arg, int unset)
 
 static int to_callback(const struct option *opt, const char *arg, int unset)
 {
-	ALLOC_GROW(extra_to, extra_to_nr + 1, extra_to_alloc);
-	extra_to[extra_to_nr++] = xstrdup(arg);
+	string_list_append(arg, &extra_to);
 	return 0;
 }
 
 static int cc_callback(const struct option *opt, const char *arg, int unset)
 {
-	ALLOC_GROW(extra_cc, extra_cc_nr + 1, extra_cc_alloc);
-	extra_cc[extra_cc_nr++] = xstrdup(arg);
+	string_list_append(arg, &extra_cc);
 	return 0;
 }
 
@@ -972,6 +961,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
+	extra_hdr.strdup_strings = 1;
+	extra_to.strdup_strings = 1;
+	extra_cc.strdup_strings = 1;
 	git_config(git_format_config, NULL);
 	init_revisions(&rev, prefix);
 	rev.commit_format = CMIT_FMT_EMAIL;
@@ -1008,29 +1000,29 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		add_signoff = xmemdupz(committer, endpos - committer + 1);
 	}
 
-	for (i = 0; i < extra_hdr_nr; i++) {
-		strbuf_addstr(&buf, extra_hdr[i]);
+	for (i = 0; i < extra_hdr.nr; i++) {
+		strbuf_addstr(&buf, extra_hdr.items[i].string);
 		strbuf_addch(&buf, '\n');
 	}
 
-	if (extra_to_nr)
+	if (extra_to.nr)
 		strbuf_addstr(&buf, "To: ");
-	for (i = 0; i < extra_to_nr; i++) {
+	for (i = 0; i < extra_to.nr; i++) {
 		if (i)
 			strbuf_addstr(&buf, "    ");
-		strbuf_addstr(&buf, extra_to[i]);
-		if (i + 1 < extra_to_nr)
+		strbuf_addstr(&buf, extra_to.items[i].string);
+		if (i + 1 < extra_to.nr)
 			strbuf_addch(&buf, ',');
 		strbuf_addch(&buf, '\n');
 	}
 
-	if (extra_cc_nr)
+	if (extra_cc.nr)
 		strbuf_addstr(&buf, "Cc: ");
-	for (i = 0; i < extra_cc_nr; i++) {
+	for (i = 0; i < extra_cc.nr; i++) {
 		if (i)
 			strbuf_addstr(&buf, "    ");
-		strbuf_addstr(&buf, extra_cc[i]);
-		if (i + 1 < extra_cc_nr)
+		strbuf_addstr(&buf, extra_cc.items[i].string);
+		if (i + 1 < extra_cc.nr)
 			strbuf_addch(&buf, ',');
 		strbuf_addch(&buf, '\n');
 	}
@@ -1239,6 +1231,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			fclose(stdout);
 	}
 	free(list);
+	string_list_clear(&extra_to, 0);
+	string_list_clear(&extra_cc, 0);
+	string_list_clear(&extra_hdr, 0);
 	if (ignore_if_in_upstream)
 		free_patch_ids(&ids);
 	return 0;
-- 
1.7.0.1.171.geb5ee
Stephen Boyd· Mar 7, 2010, 22:46 UTC · re: Stephen Boyd · lore

[PATCHv2 2/3] format-patch: add --no-cc, --no-to, and --no-add-headers

These new options allow users to override their config settings for format.cc, format.to and format.headers respectively. These options only make git ignore the config settings and any previous command line options, so you'll still have to add more command line options to add extra headers. For example,

	$ cat .git/config
	[format]
		to = Someone <someone@out.there>
	$ git format-patch -1 --no-to --to="Someone Else <else@out.there>"
would format a patch addressed to "Someone Else" and not "Someone".
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-log.c           |   25 ++++++++++++++++++-------
 t/t4014-format-patch.sh |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 7 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index ca241af..08e2ff0 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -869,19 +869,31 @@ static int inline_callback(const struct option *opt, const char *arg, int unset)
 
 static int header_callback(const struct option *opt, const char *arg, int unset)
 {
-	add_header(arg);
+	if (unset) {
+		string_list_clear(&extra_hdr, 0);
+		string_list_clear(&extra_to, 0);
+		string_list_clear(&extra_cc, 0);
+	} else {
+	    add_header(arg);
+	}
 	return 0;
 }
 
 static int to_callback(const struct option *opt, const char *arg, int unset)
 {
-	string_list_append(arg, &extra_to);
+	if (unset)
+		string_list_clear(&extra_to, 0);
+	else
+		string_list_append(arg, &extra_to);
 	return 0;
 }
 
 static int cc_callback(const struct option *opt, const char *arg, int unset)
 {
-	string_list_append(arg, &extra_cc);
+	if (unset)
+		string_list_clear(&extra_cc, 0);
+	else
+		string_list_append(arg, &extra_cc);
 	return 0;
 }
 
@@ -940,12 +952,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		  PARSE_OPT_NONEG | PARSE_OPT_NOARG },
 		OPT_GROUP("Messaging"),
 		{ OPTION_CALLBACK, 0, "add-header", NULL, "header",
-			    "add email header", PARSE_OPT_NONEG,
-			    header_callback },
+			    "add email header", 0, header_callback },
 		{ OPTION_CALLBACK, 0, "to", NULL, "email", "add To: header",
-			    PARSE_OPT_NONEG, to_callback },
+			    0, to_callback },
 		{ OPTION_CALLBACK, 0, "cc", NULL, "email", "add Cc: header",
-			    PARSE_OPT_NONEG, cc_callback },
+			    0, cc_callback },
 		OPT_STRING(0, "in-reply-to", &in_reply_to, "message-id",
 			    "make first mail a reply to <message-id>"),
 		{ OPTION_CALLBACK, 0, "attach", &rev, "boundary",
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 830ddb0..c7b6256 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -157,6 +157,44 @@ test_expect_success 'configuration To: header' '
 	grep "^To: R. E. Cipient <rcipient@example.com>\$" patch9
 '
 
+test_expect_success '--no-to overrides config.to' '
+
+	git config --replace-all format.to \
+		"R. E. Cipient <rcipient@example.com>" &&
+	git format-patch --no-to --stdout master..side |
+	sed -e "/^\$/q" >patch10 &&
+	! grep "^To: R. E. Cipient <rcipient@example.com>\$" patch10
+'
+
+test_expect_success '--no-to and --to replaces config.to' '
+
+	git config --replace-all format.to \
+		"Someone <someone@out.there>" &&
+	git format-patch --no-to --to="Someone Else <else@out.there>" \
+		--stdout master..side |
+	sed -e "/^\$/q" >patch11 &&
+	! grep "^To: Someone <someone@out.there>\$" patch11 &&
+	grep "^To: Someone Else <else@out.there>\$" patch11
+'
+
+test_expect_success '--no-cc overrides config.cc' '
+
+	git config --replace-all format.cc \
+		"C. E. Cipient <rcipient@example.com>" &&
+	git format-patch --no-cc --stdout master..side |
+	sed -e "/^\$/q" >patch12 &&
+	! grep "^Cc: C. E. Cipient <rcipient@example.com>\$" patch12
+'
+
+test_expect_success '--no-add-headers overrides config.headers' '
+
+	git config --replace-all format.headers \
+		"Header1: B. E. Cipient <rcipient@example.com>" &&
+	git format-patch --no-add-headers --stdout master..side |
+	sed -e "/^\$/q" >patch13 &&
+	! grep "^Header1: B. E. Cipient <rcipient@example.com>\$" patch13
+'
+
 test_expect_success 'multiple files' '
 
 	rm -rf patches/ &&
-- 
1.7.0.1.171.geb5ee
Stephen Boyd· Mar 7, 2010, 22:46 UTC · re: Stephen Boyd · lore

[PATCHv2 3/3] send-email: add --no-cc, --no-to, and --no-bcc

There's no way to override the sendemail.to, sendemail.cc, and sendemail.bcc config settings. Add options allowing the user to tell git to ignore the config settings and take whatever is on the command line.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 git-send-email.perl   |   14 +++++++---
 t/t9001-send-email.sh |   66 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 4 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index e05455f..d612ae8 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -47,9 +47,9 @@ git send-email [options] <file | directory | rev-list options >
 
   Composing:
     --from                  <str>  * Email From:
-    --to                    <str>  * Email To:
-    --cc                    <str>  * Email Cc:
-    --bcc                   <str>  * Email Bcc:
+    --[no-]to               <str>  * Email To:
+    --[no-]cc               <str>  * Email Cc:
+    --[no-]bcc              <str>  * Email Bcc:
     --subject               <str>  * Email "Subject:"
     --in-reply-to           <str>  * Email "In-Reply-To:"
     --annotate                     * Review each patch that will be sent in an editor.
@@ -135,7 +135,7 @@ sub unique_email_list(@);
 sub cleanup_compose_files();
 
 # Variables we fill in automatically, or via prompting:
-my (@to,@cc,@initial_cc,@bcclist,@xh,
+my (@to,$no_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,
 	$initial_reply_to,$initial_subject,@files,
 	$author,$sender,$smtp_authpass,$annotate,$compose,$time);
 
@@ -261,8 +261,11 @@ my $rc = GetOptions("sender|from=s" => \$sender,
                     "in-reply-to=s" => \$initial_reply_to,
 		    "subject=s" => \$initial_subject,
 		    "to=s" => \@to,
+		    "no-to" => \$no_to,
 		    "cc=s" => \@initial_cc,
+		    "no-cc" => \$no_cc,
 		    "bcc=s" => \@bcclist,
+		    "no-bcc" => \$no_bcc,
 		    "chain-reply-to!" => \$chain_reply_to,
 		    "smtp-server=s" => \$smtp_server,
 		    "smtp-server-port=s" => \$smtp_server_port,
@@ -305,6 +308,9 @@ sub read_config {
 
 	foreach my $setting (keys %config_settings) {
 		my $target = $config_settings{$setting};
+		next if $setting eq "to" and defined $no_to;
+		next if $setting eq "cc" and defined $no_cc;
+		next if $setting eq "bcc" and defined $no_bcc;
 		if (ref($target) eq "ARRAY") {
 			unless (@$target) {
 				my @values = Git::config(@repo, "$prefix.$setting");
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index c09f375..640b3d2 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -852,4 +852,70 @@ test_expect_success 'no warning with sendemail.chainreplyto = true' '
 	! grep "no-chain-reply-to" errors
 '
 
+test_expect_success 'sendemail.to works' '
+	git config --replace-all sendemail.to "Somebody <somebody@ex.com>" &&
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		$patches $patches >stdout &&
+	grep "To: Somebody <somebody@ex.com>" stdout
+'
+
+test_expect_success '--no-to overrides sendemail.to' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--no-to \
+		--to=nobody@example.com \
+		$patches $patches >stdout &&
+	grep "To: nobody@example.com" stdout &&
+	! grep "To: Somebody <somebody@ex.com>" stdout
+'
+
+test_expect_success 'sendemail.cc works' '
+	git config --replace-all sendemail.cc "Somebody <somebody@ex.com>" &&
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		$patches $patches >stdout &&
+	grep "Cc: Somebody <somebody@ex.com>" stdout
+'
+
+test_expect_success '--no-cc overrides sendemail.cc' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--no-cc \
+		--cc=bodies@example.com \
+		--to=nobody@example.com \
+		$patches $patches >stdout &&
+	grep "Cc: bodies@example.com" stdout &&
+	! grep "Cc: Somebody <somebody@ex.com>" stdout
+'
+
+test_expect_success 'sendemail.bcc works' '
+	git config --replace-all sendemail.bcc "Other <other@ex.com>" &&
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server relay.example.com \
+		$patches $patches >stdout &&
+	grep "RCPT TO:<other@ex.com>" stdout
+'
+
+test_expect_success '--no-bcc overrides sendemail.bcc' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--no-bcc \
+		--bcc=bodies@example.com \
+		--to=nobody@example.com \
+		--smtp-server relay.example.com \
+		$patches $patches >stdout &&
+	grep "RCPT TO:<bodies@example.com>" stdout &&
+	! grep "RCPT TO:<other@ex.com>" stdout
+'
+
 test_done
-- 
1.7.0.1.171.geb5ee
Stephen Boyd· Mar 7, 2010, 21:33 UTC · re: Junio C Hamano · lore

[PATCH 1/4] send-email: actually add bcc headers

This bug looks ancient. In fact it doesn't look like --bcc ever worked even when it was introduced in 5806324 (Add support for --bcc to git-send-email., 2006-05-29).

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 git-send-email.perl   |    7 ++++++-
 t/t9001-send-email.sh |    1 +
 2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index e05455f..3d9c832 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -856,11 +856,16 @@ sub send_message
 	if ($cc ne '') {
 		$ccline = "\nCc: $cc";
 	}
+	my $bcc = join(",\n\t", unique_email_list(@bcclist));
+	my $bccline = "";
+	if ($bcc ne '') {
+		$bccline = "\nBcc: $bcc";
+	}
 	my $sanitized_sender = sanitize_address($sender);
 	make_message_id() unless defined($message_id);
 
 	my $header = "From: $sanitized_sender
-To: $to${ccline}
+To: $to${ccline}${bccline}
 Subject: $subject
 Date: $date
 Message-Id: $message_id
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index c09f375..db91721 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -149,6 +149,7 @@ Cc: cc@example.com,
 	A <author@example.com>,
 	One <one@example.com>,
 	two@example.com
+Bcc: bcc@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
-- 
1.7.0.1.171.geb5ee
Stephen Boyd· Mar 7, 2010, 21:53 UTC · re: Stephen Boyd · lore

Re: [PATCH 1/4] send-email: actually add bcc headers

On 03/07/2010 01:33 PM, Stephen Boyd wrote:
Show 6 quoted lines
> This bug looks ancient. In fact it doesn't look like --bcc ever worked
> even when it was introduced in 5806324 (Add support for --bcc to
> git-send-email., 2006-05-29).
>
> Signed-off-by: Stephen Boyd <bebarino@gmail.com>
>   

Haha. Nevermind this one, I'm blind and can't read. Need to figure out a way to test the bcc list though.

Stephen Boyd· Mar 7, 2010, 21:33 UTC · re: Junio C Hamano · lore

[PATCH 2/4] format-patch: use a string_list for headers

In the next patch we'll need to clear the header lists if the user specifies --no-add-headers or --no-to or --no-cc. This actually cuts down on the code a bit too.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
I had a patch like this but using strbuf's instead. I couldn't find it...
 builtin-log.c |   70 +++++++++++++++++++++++++-------------------------------
 1 files changed, 31 insertions(+), 39 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 5d23a67..dd8369f 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -458,35 +458,28 @@ static int auto_number = 1;
 
 static char *default_attach = NULL;
 
-static char **extra_hdr;
-static int extra_hdr_nr;
-static int extra_hdr_alloc;
-
-static char **extra_to;
-static int extra_to_nr;
-static int extra_to_alloc;
-
-static char **extra_cc;
-static int extra_cc_nr;
-static int extra_cc_alloc;
+static struct string_list extra_hdr = { .strdup_strings = 1 };
+static struct string_list extra_to = { .strdup_strings = 1 };
+static struct string_list extra_cc = { .strdup_strings = 1 };
 
 static void add_header(const char *value)
 {
+	struct string_list_item *i;
 	int len = strlen(value);
 	while (len && value[len - 1] == '\n')
 		len--;
+
 	if (!strncasecmp(value, "to: ", 4)) {
-		ALLOC_GROW(extra_to, extra_to_nr + 1, extra_to_alloc);
-		extra_to[extra_to_nr++] = xstrndup(value + 4, len - 4);
-		return;
+		i = string_list_append(value + 4, &extra_to);
+		len -= 4;
+	} else if (!strncasecmp(value, "cc: ", 4)) {
+		i = string_list_append(value + 4, &extra_cc);
+		len -= 4;
+	} else {
+		i =string_list_append(value, &extra_hdr);
 	}
-	if (!strncasecmp(value, "cc: ", 4)) {
-		ALLOC_GROW(extra_cc, extra_cc_nr + 1, extra_cc_alloc);
-		extra_cc[extra_cc_nr++] = xstrndup(value + 4, len - 4);
-		return;
-	}
-	ALLOC_GROW(extra_hdr, extra_hdr_nr + 1, extra_hdr_alloc);
-	extra_hdr[extra_hdr_nr++] = xstrndup(value, len);
+
+	i->string[len] = '\0';
 }
 
 #define THREAD_SHALLOW 1
@@ -507,15 +500,13 @@ static int git_format_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "format.to")) {
 		if (!value)
 			return config_error_nonbool(var);
-		ALLOC_GROW(extra_to, extra_to_nr + 1, extra_to_alloc);
-		extra_to[extra_to_nr++] = xstrdup(value);
+		string_list_append(value, &extra_to);
 		return 0;
 	}
 	if (!strcmp(var, "format.cc")) {
 		if (!value)
 			return config_error_nonbool(var);
-		ALLOC_GROW(extra_cc, extra_cc_nr + 1, extra_cc_alloc);
-		extra_cc[extra_cc_nr++] = xstrdup(value);
+		string_list_append(value, &extra_cc);
 		return 0;
 	}
 	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
@@ -884,15 +875,13 @@ static int header_callback(const struct option *opt, const char *arg, int unset)
 
 static int to_callback(const struct option *opt, const char *arg, int unset)
 {
-	ALLOC_GROW(extra_to, extra_to_nr + 1, extra_to_alloc);
-	extra_to[extra_to_nr++] = xstrdup(arg);
+	string_list_append(arg, &extra_to);
 	return 0;
 }
 
 static int cc_callback(const struct option *opt, const char *arg, int unset)
 {
-	ALLOC_GROW(extra_cc, extra_cc_nr + 1, extra_cc_alloc);
-	extra_cc[extra_cc_nr++] = xstrdup(arg);
+	string_list_append(arg, &extra_cc);
 	return 0;
 }
 
@@ -1008,29 +997,29 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		add_signoff = xmemdupz(committer, endpos - committer + 1);
 	}
 
-	for (i = 0; i < extra_hdr_nr; i++) {
-		strbuf_addstr(&buf, extra_hdr[i]);
+	for (i = 0; i < extra_hdr.nr; i++) {
+		strbuf_addstr(&buf, extra_hdr.items[i].string);
 		strbuf_addch(&buf, '\n');
 	}
 
-	if (extra_to_nr)
+	if (extra_to.nr)
 		strbuf_addstr(&buf, "To: ");
-	for (i = 0; i < extra_to_nr; i++) {
+	for (i = 0; i < extra_to.nr; i++) {
 		if (i)
 			strbuf_addstr(&buf, "    ");
-		strbuf_addstr(&buf, extra_to[i]);
-		if (i + 1 < extra_to_nr)
+		strbuf_addstr(&buf, extra_to.items[i].string);
+		if (i + 1 < extra_to.nr)
 			strbuf_addch(&buf, ',');
 		strbuf_addch(&buf, '\n');
 	}
 
-	if (extra_cc_nr)
+	if (extra_cc.nr)
 		strbuf_addstr(&buf, "Cc: ");
-	for (i = 0; i < extra_cc_nr; i++) {
+	for (i = 0; i < extra_cc.nr; i++) {
 		if (i)
 			strbuf_addstr(&buf, "    ");
-		strbuf_addstr(&buf, extra_cc[i]);
-		if (i + 1 < extra_cc_nr)
+		strbuf_addstr(&buf, extra_cc.items[i].string);
+		if (i + 1 < extra_cc.nr)
 			strbuf_addch(&buf, ',');
 		strbuf_addch(&buf, '\n');
 	}
@@ -1239,6 +1228,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			fclose(stdout);
 	}
 	free(list);
+	string_list_clear(&extra_to, 0);
+	string_list_clear(&extra_cc, 0);
+	string_list_clear(&extra_hdr, 0);
 	if (ignore_if_in_upstream)
 		free_patch_ids(&ids);
 	return 0;
-- 
1.7.0.1.171.geb5ee
Erik Faye-Lund· Mar 7, 2010, 21:44 UTC · re: Stephen Boyd · lore

Re: [PATCH 2/4] format-patch: use a string_list for headers

On Sun, Mar 7, 2010 at 10:33 PM, Stephen Boyd <bebarino@gmail.com> wrote:
> +static struct string_list extra_hdr = { .strdup_strings = 1 };
> +static struct string_list extra_to = { .strdup_strings = 1 };
> +static struct string_list extra_cc = { .strdup_strings = 1 };
>

Do we really use this C99 feature (designated initializers)? I think it will break MSVC builds, at least... Perhaps some other non-gcc platforms as well?

-- 
Erik "kusma" Faye-Lund
Stephen Boyd· Mar 7, 2010, 21:54 UTC · re: Erik Faye-Lund · lore

Re: [PATCH 2/4] format-patch: use a string_list for headers

On 03/07/2010 01:44 PM, Erik Faye-Lund wrote:
Show 11 quoted lines
> On Sun, Mar 7, 2010 at 10:33 PM, Stephen Boyd <bebarino@gmail.com> wrote:
>   
>> +static struct string_list extra_hdr = { .strdup_strings = 1 };
>> +static struct string_list extra_to = { .strdup_strings = 1 };
>> +static struct string_list extra_cc = { .strdup_strings = 1 };
>>
>>     
> Do we really use this C99 feature (designated initializers)? I think
> it will break MSVC builds, at least... Perhaps some other non-gcc
> platforms as well?
>   
Figured as much. Not a problem to fix. Thanks.
Johannes Schindelin· Mar 7, 2010, 22:13 UTC · re: Erik Faye-Lund · lore

Re: [PATCH 2/4] format-patch: use a string_list for headers

Hi,
On Sun, 7 Mar 2010, Erik Faye-Lund wrote:
Show 9 quoted lines
> On Sun, Mar 7, 2010 at 10:33 PM, Stephen Boyd <bebarino@gmail.com> wrote:
> > +static struct string_list extra_hdr = { .strdup_strings = 1 };
> > +static struct string_list extra_to = { .strdup_strings = 1 };
> > +static struct string_list extra_cc = { .strdup_strings = 1 };
> >
> 
> Do we really use this C99 feature (designated initializers)? I think
> it will break MSVC builds, at least... Perhaps some other non-gcc
> platforms as well?
http://repo.or.cz/w/git.git/blob?f=Documentation/SubmittingPatches#l84

Ciao, Dscho

Stephen Boyd· Mar 7, 2010, 21:33 UTC · re: Junio C Hamano · lore

[PATCH 3/4] format-patch: add --no-cc, --no-to, and --no-add-headers

These new options allow users to override their config settings for format.cc, format.to and format.headers respectively. These options only make git ignore the config settings and any previous command line options, so you'll still have to add more command line options to add extra headers. For example,

	$ cat .git/config
	[format]
		to = Someone <someone@out.there>
	$ git format-patch -1 --no-to --to="Someone Else <else@out.there>"
would format a patch addressed to "Someone Else" and not "Someone".
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-log.c           |   25 ++++++++++++++++++-------
 t/t4014-format-patch.sh |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 7 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index dd8369f..00cddcc 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -869,19 +869,31 @@ static int inline_callback(const struct option *opt, const char *arg, int unset)
 
 static int header_callback(const struct option *opt, const char *arg, int unset)
 {
-	add_header(arg);
+	if (unset) {
+		string_list_clear(&extra_hdr, 0);
+		string_list_clear(&extra_to, 0);
+		string_list_clear(&extra_cc, 0);
+	} else {
+	    add_header(arg);
+	}
 	return 0;
 }
 
 static int to_callback(const struct option *opt, const char *arg, int unset)
 {
-	string_list_append(arg, &extra_to);
+	if (unset)
+		string_list_clear(&extra_to, 0);
+	else
+		string_list_append(arg, &extra_to);
 	return 0;
 }
 
 static int cc_callback(const struct option *opt, const char *arg, int unset)
 {
-	string_list_append(arg, &extra_cc);
+	if (unset)
+		string_list_clear(&extra_cc, 0);
+	else
+		string_list_append(arg, &extra_cc);
 	return 0;
 }
 
@@ -940,12 +952,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		  PARSE_OPT_NONEG | PARSE_OPT_NOARG },
 		OPT_GROUP("Messaging"),
 		{ OPTION_CALLBACK, 0, "add-header", NULL, "header",
-			    "add email header", PARSE_OPT_NONEG,
-			    header_callback },
+			    "add email header", 0, header_callback },
 		{ OPTION_CALLBACK, 0, "to", NULL, "email", "add To: header",
-			    PARSE_OPT_NONEG, to_callback },
+			    0, to_callback },
 		{ OPTION_CALLBACK, 0, "cc", NULL, "email", "add Cc: header",
-			    PARSE_OPT_NONEG, cc_callback },
+			    0, cc_callback },
 		OPT_STRING(0, "in-reply-to", &in_reply_to, "message-id",
 			    "make first mail a reply to <message-id>"),
 		{ OPTION_CALLBACK, 0, "attach", &rev, "boundary",
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 830ddb0..c7b6256 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -157,6 +157,44 @@ test_expect_success 'configuration To: header' '
 	grep "^To: R. E. Cipient <rcipient@example.com>\$" patch9
 '
 
+test_expect_success '--no-to overrides config.to' '
+
+	git config --replace-all format.to \
+		"R. E. Cipient <rcipient@example.com>" &&
+	git format-patch --no-to --stdout master..side |
+	sed -e "/^\$/q" >patch10 &&
+	! grep "^To: R. E. Cipient <rcipient@example.com>\$" patch10
+'
+
+test_expect_success '--no-to and --to replaces config.to' '
+
+	git config --replace-all format.to \
+		"Someone <someone@out.there>" &&
+	git format-patch --no-to --to="Someone Else <else@out.there>" \
+		--stdout master..side |
+	sed -e "/^\$/q" >patch11 &&
+	! grep "^To: Someone <someone@out.there>\$" patch11 &&
+	grep "^To: Someone Else <else@out.there>\$" patch11
+'
+
+test_expect_success '--no-cc overrides config.cc' '
+
+	git config --replace-all format.cc \
+		"C. E. Cipient <rcipient@example.com>" &&
+	git format-patch --no-cc --stdout master..side |
+	sed -e "/^\$/q" >patch12 &&
+	! grep "^Cc: C. E. Cipient <rcipient@example.com>\$" patch12
+'
+
+test_expect_success '--no-add-headers overrides config.headers' '
+
+	git config --replace-all format.headers \
+		"Header1: B. E. Cipient <rcipient@example.com>" &&
+	git format-patch --no-add-headers --stdout master..side |
+	sed -e "/^\$/q" >patch13 &&
+	! grep "^Header1: B. E. Cipient <rcipient@example.com>\$" patch13
+'
+
 test_expect_success 'multiple files' '
 
 	rm -rf patches/ &&
-- 
1.7.0.1.171.geb5ee
Stephen Boyd· Mar 7, 2010, 21:33 UTC · re: Junio C Hamano · lore

[PATCH 4/4] send-email: add --no-cc, --no-to, and --no-bcc

There's no way to override the sendemail.to, sendemail.cc, and sendemail.bcc config settings. Add options allowing the user to tell git to ignore the config settings and take whatever is on the command line.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 git-send-email.perl   |   14 ++++++++++----
 t/t9001-send-email.sh |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 3d9c832..0a91f4a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -47,9 +47,9 @@ git send-email [options] <file | directory | rev-list options >
 
   Composing:
     --from                  <str>  * Email From:
-    --to                    <str>  * Email To:
-    --cc                    <str>  * Email Cc:
-    --bcc                   <str>  * Email Bcc:
+    --[no-]to               <str>  * Email To:
+    --[no-]cc               <str>  * Email Cc:
+    --[no-]bcc              <str>  * Email Bcc:
     --subject               <str>  * Email "Subject:"
     --in-reply-to           <str>  * Email "In-Reply-To:"
     --annotate                     * Review each patch that will be sent in an editor.
@@ -135,7 +135,7 @@ sub unique_email_list(@);
 sub cleanup_compose_files();
 
 # Variables we fill in automatically, or via prompting:
-my (@to,@cc,@initial_cc,@bcclist,@xh,
+my (@to,$no_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,
 	$initial_reply_to,$initial_subject,@files,
 	$author,$sender,$smtp_authpass,$annotate,$compose,$time);
 
@@ -261,8 +261,11 @@ my $rc = GetOptions("sender|from=s" => \$sender,
                     "in-reply-to=s" => \$initial_reply_to,
 		    "subject=s" => \$initial_subject,
 		    "to=s" => \@to,
+		    "no-to" => \$no_to,
 		    "cc=s" => \@initial_cc,
+		    "no-cc" => \$no_cc,
 		    "bcc=s" => \@bcclist,
+		    "no-bcc" => \$no_bcc,
 		    "chain-reply-to!" => \$chain_reply_to,
 		    "smtp-server=s" => \$smtp_server,
 		    "smtp-server-port=s" => \$smtp_server_port,
@@ -305,6 +308,9 @@ sub read_config {
 
 	foreach my $setting (keys %config_settings) {
 		my $target = $config_settings{$setting};
+		next if $setting eq "to" and defined $no_to;
+		next if $setting eq "cc" and defined $no_cc;
+		next if $setting eq "bcc" and defined $no_bcc;
 		if (ref($target) eq "ARRAY") {
 			unless (@$target) {
 				my @values = Git::config(@repo, "$prefix.$setting");
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index db91721..60bca7e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -853,4 +853,42 @@ test_expect_success 'no warning with sendemail.chainreplyto = true' '
 	! grep "no-chain-reply-to" errors
 '
 
+test_expect_success '--no-to overrides sendemail.to' '
+	git config --replace-all sendemail.to "Somebody <somebody@ex.com>" &&
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--no-to \
+		--to=nobody@example.com \
+		$patches $patches >stdout &&
+	grep "To: nobody@example.com" stdout &&
+	! grep "To: Somebody <somebody@ex.com>" stdout
+'
+
+test_expect_success '--no-cc overrides sendemail.cc' '
+	git config --replace-all sendemail.cc "Somebody <somebody@ex.com>" &&
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--no-cc \
+		--cc=bodies@example.com \
+		--to=nobody@example.com \
+		$patches $patches >stdout &&
+	grep "Cc: bodies@example.com" stdout &&
+	! grep "Cc: Somebody <somebody@ex.com>" stdout
+'
+
+test_expect_success '--no-bcc overrides sendemail.bcc' '
+	git config --replace-all sendemail.bcc "Somebody <somebody@ex.com>" &&
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--no-bcc \
+		--bcc=bodies@example.com \
+		--to=nobody@example.com \
+		$patches $patches >stdout &&
+	grep "Bcc: bodies@example.com" stdout &&
+	! grep "Bcc: Somebody <somebody@ex.com>" stdout
+'
+
 test_done
-- 
1.7.0.1.171.geb5ee
Steven Drake· Mar 10, 2010, 03:53 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add tests for git format-patch --to and format.to config option

On Sun, 7 Mar 2010, Junio C Hamano wrote:
Show 13 quoted lines
> Stephen Boyd <bebarino@gmail.com> writes:
> 
> > ... We have the
> > option of making them consistent with the rest of git with a little bit
> > of work. If you say --no-cc or --no-add-headers or --no-to the
> > respective config should be overriden. If you say --to or --cc or
> > --add-headers it should be appended. I doubt anyone would find that
> > surprising since --no-* doesn't do anything right now.
> 
> That sounds like a sensible and practical way out, as it won't break
> existing setup that expects the additive behaviour these two command
> somehow ended up with, while allowing --no-* to override the config when
> necessary.
I agree with that.
-- 
Steven

← back to recent threads