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

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

[Junio C Hamano]

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.

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

[Adam Simpkins]

On Wed, Mar 03, 2010 at 04:02:20PM -0800, Junio C Hamano wrote:
> 
> * 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

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

[Björn Gustavsson]

On Thu, Mar 4, 2010 at 1:02 AM, Junio C Hamano <gitster@pobox.com> wrote:

> * 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

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

[Junio C Hamano]

Björn Gustavsson <bgustavsson@gmail.com> writes:

> 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.

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

[Tay Ray Chuan]

Hi,

On Thu, Mar 4, 2010 at 8:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * 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

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

[Junio C Hamano]

Tay Ray Chuan <rctay89@gmail.com> writes:

> 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.

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> * 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.

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

[Junio C Hamano]

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?

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

[Jonathan Nieder]

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.

> 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.

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

[Christian Couder]

On Friday 05 March 2010 17:25:21 Jonathan Nieder wrote:
>
> 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.

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

[Christian Couder]

On Friday 05 March 2010 01:49:21 Junio C Hamano wrote:
> 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.

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

[Thomas Rast]

On Thursday 04 March 2010 01:02:20 Junio C Hamano wrote:
> 
> * 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

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

[Mark Lodato]

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.

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

[Mark Lodato]

On Thu, Mar 4, 2010 at 8:30 PM, Mark Lodato <lodatom@gmail.com> wrote:
> 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.

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

[Junio C Hamano]

Mark Lodato <lodatom@gmail.com> writes:

> 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.

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

[Miklos Vajna]

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:
> * 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

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

[Junio C Hamano]

Miklos Vajna <vmiklos@frugalware.org> writes:

> +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.

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

[Miklos Vajna]

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

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

[Stephen Boyd]

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?

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

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

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.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> 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.

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

[Stephen Boyd]

On 03/06/2010 07:42 PM, Junio C Hamano wrote:
> 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.

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

[Junio C Hamano]

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.

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

[Stephen Boyd]

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(-)

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

[Stephen Boyd]

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(-)

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

[Stephen Boyd]

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

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

[Stephen Boyd]

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

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

[Stephen Boyd]

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

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

[Junio C Hamano]

Thanks; will queue.

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

[Stephen Boyd]

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

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

[Stephen Boyd]

On 03/07/2010 01:33 PM, Stephen Boyd wrote:
> 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.

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

[Stephen Boyd]

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

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

[Erik Faye-Lund]

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

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

[Stephen Boyd]

On 03/07/2010 01:44 PM, Erik Faye-Lund wrote:
> 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.

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

[Johannes Schindelin]

Hi,

On Sun, 7 Mar 2010, Erik Faye-Lund wrote:

> 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

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

[Stephen Boyd]

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

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

[Stephen Boyd]

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

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

[Steven Drake]

On Sun, 7 Mar 2010, Junio C Hamano wrote:
> 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