What's cooking in git.git (Aug 2009, #06; Sun, 30)

What's cooking in git.git (Aug 2009, #06; Sun, 30)

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

After the 1.6.5 cycle, the next release will be 1.7.0, and we will push
out the planned "push safety" change.  1.7.0 would be a good time to
introduce "justifiable" changes that are not strictly backward compatible.

During 1.6.5 cycle, 'next' will hold topics meant for 1.6.5 and 1.7.0.

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

* wl/insta-mongoose (2009-08-21) 1 commit
  (merged to 'next' on 2009-08-25 at da1d566)
 + Add support for the Mongoose web server.

* as/maint-graph-interesting-fix (2009-08-21) 2 commits.
  (merged to 'next' on 2009-08-25 at 9d5e215)
 + Add tests for rev-list --graph with options that simplify history
 + graph API: fix bug in graph_is_interesting()

* jc/maint-unpack-objects-strict (2009-08-13) 1 commit.
  (merged to 'next' on 2009-08-23 at 38eb750)
 + Fix "unpack-objects --strict"

* jh/submodule-foreach (2009-08-20) 9 commits
  (merged to 'next' on 2009-08-20 at 671bea4)
 + git clone: Add --recursive to automatically checkout (nested) submodules
 + t7407: Use 'rev-parse --short' rather than bash's substring expansion notation
  (merged to 'next' on 2009-08-18 at f4a881d)
 + git submodule status: Add --recursive to recurse into nested submodules
 + git submodule update: Introduce --recursive to update nested submodules
 + git submodule foreach: Add --recursive to recurse into nested submodules
 + git submodule foreach: test access to submodule name as '$name'
 + Add selftest for 'git submodule foreach'
 + git submodule: Cleanup usage string and add option parsing to cmd_foreach()
 + git submodule foreach: Provide access to submodule name, as '$name'

* lt/block-sha1 (2009-08-17) 4 commits
  (merged to 'next' on 2009-08-18 at 67a1ce8)
 + remove ARM and Mozilla SHA1 implementations
 + block-sha1: guard gcc extensions with __GNUC__
 + make sure byte swapping is optimal for git
 + block-sha1: make the size member first in the context struct

* np/maint-1.6.3-deepen (2009-08-24) 1 commit.
  (merged to 'next' on 2009-08-25 at 8e383d4)
 + fix simple deepening of a repo

* jk/maint-1.6.3-checkout-unborn (2009-08-24) 1 commit.
  (merged to 'next' on 2009-08-25 at 5f29625)
 + checkout: do not imply "-f" on unborn branches

* mm/reset-report (2009-08-21) 2 commits
  (merged to 'next' on 2009-08-25 at f2a4424)
 + reset: make the reminder output consistent with "checkout"
 + Rename REFRESH_SAY_CHANGED to REFRESH_IN_PORCELAIN.

* jc/shortstatus (2009-08-15) 11 commits
  (merged to 'next' on 2009-08-15 at 7e40766)
 + git commit --dry-run -v: show diff in color when asked
 + Documentation/git-commit.txt: describe --dry-run
  (merged to 'next' on 2009-08-12 at 53bda17)
 + wt-status: collect untracked files in a separate "collect" phase
 + Make git_status_config() file scope static to builtin-commit.c
 + wt-status: move wt_status_colors[] into wt_status structure
 + wt-status: move many global settings to wt_status structure
 + commit: --dry-run
  (merged to 'next' on 2009-08-06 at fe8cb94)
 + status: show worktree status of conflicted paths separately
 + wt-status.c: rework the way changes to the index and work tree are summarized
 + diff-index: keep the original index intact
 + diff-index: report unmerged new entries
 (this branch is used by jc/1.7.0-status.)

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

* jc/upload-pack-hook (2009-08-28) 2 commits
 - upload-pack: feed "kind [clone|fetch]" to post-upload-pack hook
 - upload-pack: add a trigger for post-upload-pack hook

I do not know if the distinction between fetching some but not all refs
and fetching full set of refs into an empty repository is something worth
making, so in that sense the tip commit is somewhat iffy.

One reason this series makes me somewhat uneasy is that Tom, the original
starter of the discussion went dark after sending a proposed patch.  Maybe
he has been too busy, but I have been hoping that GitHub as a stakeholder
has somebody who monitors the list when he is not available.

Does anybody from GitHub have any input?  Is there something that needs to
be improved to fill GitHub's needs?  Does GitHub want to stick to its own
fork, and were all these discussions for improvements unwanted?

* jk/clone-b (2009-08-26) 1 commit
  (merged to 'next' on 2009-08-30 at 10a68d1)
 + clone: add --branch option to select a different HEAD

* pk/import-dirs (2009-08-24) 1 commit
 - Add script for importing bits-and-pieces to Git.

This version makes me suspect that the author might regret the choice of
the import format that does not allow escaping of paths, nor does not
allow leading blanks for readability without changing semantics, both of
which make it somewhat limiting and error prone.  These issues will be
hard to rectify without breaking the backward compatibility, for a tool
that could otherwise turn out to be useful.

As a contrib/ material, I probably shouldn't be too worried about these
issues, but I am keeping this out of 'next' for now, just in case the
author chooses to polish the usability of the tool for general audience.

It is a different story if the submission was just throwing out a one-time
hack in the open in the hope that some other people might find it useful,
but without any intention of maintaining it.  But then I do not have a
strong reason to keep this in my tree, either.  The mailing list archive
is a more suitable storage media for such a patch.

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

* jh/notes (2009-08-27) 12 commits.
 - Add '%N'-format for pretty-printing commit notes
 - Add flags to get_commit_notes() to control the format of the note string
 - notes.c: Implement simple memory pooling of leaf nodes
 - Selftests verifying semantics when loading notes trees with various fanouts
 - Teach the notes lookup code to parse notes trees with various fanout schemes
 - t3302-notes-index-expensive: Speed up create_repo()
 - fast-import: Add support for importing commit notes
 - Teach "-m <msg>" and "-F <file>" to "git notes edit"
 - Add an expensive test for git-notes
 - Speed up git notes lookup
 - Add a script to edit/inspect notes
 - Introduce commit notes

I heard the cvs-helper series depends on this one.  It seems that the
fan-out strategy is being rethought?

* js/stash-dwim (2009-07-27) 1 commit.
  (merged to 'next' on 2009-08-16 at 67896c4)
 + Make 'git stash -k' a short form for 'git stash save --keep-index'
 (this branch is used by tr/reset-checkout-patch.)

* tr/reset-checkout-patch (2009-08-27) 9 commits.
  (merged to 'next' on 2009-08-27 at d314281)
 + Make test case number unique
  (merged to 'next' on 2009-08-18 at e465bb3)
 + tests: disable interactive hunk selection tests if perl is not available
  (merged to 'next' on 2009-08-16 at 67896c4)
 + DWIM 'git stash save -p' for 'git stash -p'
 + Implement 'git stash save --patch'
 + Implement 'git checkout --patch'
 + Implement 'git reset --patch'
 + builtin-add: refactor the meat of interactive_add()
 + Add a small patch-mode testing library
 + git-apply--interactive: Refactor patch mode code
 (this branch uses js/stash-dwim.)

There was a discussion on better DWIMmery for the above two topics to (1)
forbid "git stash save --anything-with-dash" and (2) redirect with any
option "git stash --opt" to "git stash save --opt", to keep it flexible
and safe at the same time.  I think it is a sane thing to do, but nothing
has happened lately.

* db/vcs-helper (2009-08-09) 17 commits
 - Allow helpers to request marks for fast-import
 - Allow helpers to report in "list" command that the ref is unchanged
 - Add support for "import" helper command
 - transport-helper_init(): fix a memory leak in error path
 - Add a config option for remotes to specify a foreign vcs
 - Allow programs to not depend on remotes having urls
 - Allow fetch to modify refs
 - Use a function to determine whether a remote is valid
 - Use a clearer style to issue commands to remote helpers
  (merged to 'next' on 2009-08-07 at f3533ba)
 + Makefile: install hardlinks for git-remote-<scheme> supported by libcurl if possible
 + Makefile: do not link three copies of git-remote-* programs
 + Makefile: git-http-fetch does not need expat
  (merged to 'next' on 2009-08-06 at 15da79d)
 + http-fetch: Fix Makefile dependancies
 + Add transport native helper executables to .gitignore
  (merged to 'next' on 2009-08-05 at 33d491e)
 + git-http-fetch: not a builtin
 + Use an external program to implement fetching with curl
 + Add support for external programs for handling native fetches
 (this branch is used by jh/cvs-helper.)

We had a few messages on what the list consensus was with this series.  My
impression, after going back to the archive, is that there wasn't.

* jn/gitweb-blame (2009-08-06) 3 commits
 - gitweb: Create links leading to 'blame_incremental' using JavaScript
 - gitweb: Incremental blame (WIP)
 - gitweb: Add optional "time to generate page" info in footer

Ajax-y blame WIP

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

* je/send-email-no-subject (2009-08-05) 1 commit
  (merged to 'next' on 2009-08-30 at b6455c2)
 + send-email: confirm on empty mail subjects

The existing tests to covers the positive case (i.e. as long as the user
says "yes" to the "do you really want to send this message that lacks
subject", the message is sent) of this feature, but the feature itself
needs its own test to verify the negative case (i.e. does it correctly
stop if the user says "no"?)

* lt/approxidate (2009-08-30) 6 commits
  (merged to 'next' on 2009-08-30 at e016e3d)
 + fix approxidate parsing of relative months and years
 + tests: add date printing and parsing tests
 + refactor test-date interface
 + Add date formatting and parsing functions relative to a given time
  (merged to 'next' on 2009-08-26 at 62853f9)
 + Further 'approxidate' improvements
 + Improve on 'approxidate'

Fixes a few "reasonably formatted but thus-far misparsed" date strings.
With tests by Peff, this should be ready for -rc0.

* mr/gitweb-snapshot (2009-08-25) 3 commits
  (merged to 'next' on 2009-08-30 at e4edd0b)
 + gitweb: add t9501 tests for checking HTTP status codes
 + gitweb: split test suite into library and tests
 + gitweb: improve snapshot error handling

* jc/mailinfo-scissors (2009-08-26) 5 commits
  (merged to 'next' on 2009-08-30 at 5fc6248)
 + mailinfo.scissors: new configuration
 + am/mailinfo: Disable scissors processing by default
 + Documentation: describe the scissors mark support of "git am"
 + Teach mailinfo to ignore everything before -- >8 -- mark
 + builtin-mailinfo.c: fix confusing internal API to mailinfo()

I didn't pick up the patch to simplify the definition of scissors. I do
not have strong opinion on it either way, but the list would hopefully
decide it before too long.

* tf/diff-whitespace-incomplete-line (2009-08-23) 2 commits.
  (merged to 'next' on 2009-08-26 at 4fc7784)
 + xutils: Fix xdl_recmatch() on incomplete lines
 + xutils: Fix hashing an incomplete line with whitespaces at the end

Will merge.

* cc/sequencer-rebase-i (2009-08-28) 15 commits
 - rebase -i: use "git sequencer--helper --cherry-pick"
 - sequencer: add "--cherry-pick" option to "git sequencer--helper"
 - sequencer: add "do_commit()" and related functions working on "next_commit"
 - pick: libify "pick_help_msg()"
 - revert: libify cherry-pick and revert functionnality
 - rebase -i: use "git sequencer--helper --fast-forward"
 - sequencer: let "git sequencer--helper" callers set "allow_dirty"
 - sequencer: add "--fast-forward" option to "git sequencer--helper"
 - sequencer: add "do_fast_forward()" to perform a fast forward
 - rebase -i: use "git sequencer--helper --reset-hard"
 - sequencer: add "--reset-hard" option to "git sequencer--helper"
 - sequencer: add "reset_almost_hard()" and related functions
 - rebase -i: use "git sequencer--helper --make-patch"
 - sequencer: add "make_patch" function to save a patch
 - sequencer: add "builtin-sequencer--helper.c"

Migrating "rebase -i" bit by bit to C.

* jh/cvs-helper (2009-08-18) 7 commits
 - More fixes to the git-remote-cvs installation procedure
 - Fix the Makefile-generated path to the git_remote_cvs package in git-remote-cvs
 - Add simple selftests of git-remote-cvs functionality
 - git-remote-cvs: Remote helper program for CVS repositories
 - 2/2: Add Python support library for CVS remote helper
 - 1/2: Add Python support library for CVS remote helper
 - Basic build infrastructure for Python scripts
 (this branch uses db/vcs-helper.)

Builds on db/vcs-helper (which is stalled, so this cannot move further at
the moment).  There is a re-roll planned, so I did not pick up test fixes
from Brandon myself.

* sr/gfi-options (2009-08-27) 6 commits
 - fast-import: test the new option command
 - fast-import: add option command
 - fast-import: test the new feature command
 - fast-import: add feature command
 - fast-import: put marks reading in it's own function
 - fast-import: put option parsing code in separate functions

Re-rolled, based on an off-list discussion I was/am not aware of.
Looked ready for 'next'.

* nd/sparse (2009-08-20) 19 commits
 - sparse checkout: inhibit empty worktree
 - Add tests for sparse checkout
 - read-tree: add --no-sparse-checkout to disable sparse checkout support
 - unpack-trees(): ignore worktree check outside checkout area
 - unpack_trees(): apply $GIT_DIR/info/sparse-checkout to the final index
 - unpack-trees(): "enable" sparse checkout and load $GIT_DIR/info/sparse-checkout
 - unpack-trees.c: generalize verify_* functions
 - unpack-trees(): add CE_WT_REMOVE to remove on worktree alone
 - Introduce "sparse checkout"
 - dir.c: export excluded_1() and add_excludes_from_file_1()
 - excluded_1(): support exclude files in index
 - unpack-trees(): carry skip-worktree bit over in merged_entry()
 - Read .gitignore from index if it is skip-worktree
 - Avoid writing to buffer in add_excludes_from_file_1()
 - Teach Git to respect skip-worktree bit (writing part)
 - Teach Git to respect skip-worktree bit (reading part)
 - Introduce "skip-worktree" bit in index, teach Git to get/set this bit
 - Add test-index-version
 - update-index: refactor mark_valid() in preparation for new options

--------------------------------------------------
[For 1.7.0]

* jc/1.7.0-status (2009-08-15) 3 commits
  (merged to 'next' on 2009-08-22 at b3507bb)
 + git status: not "commit --dry-run" anymore
 + git stat -s: short status output
 + git stat: the beginning of "status that is not a dry-run of commit"

With this, "git status" is no longer "git commit --preview".

* jc/1.7.0-send-email-no-thread-default (2009-08-22) 1 commit
  (merged to 'next' on 2009-08-22 at 5106de8)
 + send-email: make --no-chain-reply-to the default

* jc/1.7.0-diff-whitespace-only-status (2009-08-30) 4 commits.
  (merged to 'next' on 2009-08-30 at 0623572)
 + diff.c: fix typoes in comments
  (merged to 'next' on 2009-08-27 at 81fb2bd)
 + Make test case number unique
  (merged to 'next' on 2009-08-02 at 9c08420)
 + diff: Rename QUIET internal option to QUICK
 + diff: change semantics of "ignore whitespace" options

This changes exit code from "git diff --ignore-whitespace" and friends
when there is no actual output.  It is a backward incompatible change, but
we could argue that it is a bugfix.

* jc/1.7.0-push-safety (2009-02-09) 2 commits
  (merged to 'next' on 2009-08-02 at 38b82fe)
 + Refuse deleting the current branch via push
 + Refuse updating the current branch in a non-bare repository via push

--------------------------------------------------
[I have been too busy to purge these]

* jc/log-tz (2009-03-03) 1 commit.
 - Allow --date=local --date=other-format to work as expected

Maybe some people care about this.  I dunno.

* jc/mailinfo-remove-brackets (2009-07-15) 1 commit.
 - mailinfo: -b option keeps [bracketed] strings that is not a [PATCH] marker

Maybe some people care about this.  I dunno.

* ar/maint-1.6.2-merge-recursive-d-f (2009-05-11) 2 commits.
 . Fix for a merge where a branch has an F->D transition
 . Add a reminder test case for a merge with F/D transition

* jc/merge-convert (2009-01-26) 1 commit.
 . git-merge-file: allow converting the results for the work tree

* lt/read-directory (2009-05-15) 3 commits.
 . Add initial support for pathname conversion to UTF-8
 . read_directory(): infrastructure for pathname character set conversion
 . Add 'fill_directory()' helper function for directory traversal

* ps/blame (2009-03-12) 1 commit.
 . blame.c: start libifying the blame infrastructure

* pb/tracking (2009-07-16) 7 commits.
 . branch.c: if remote is not config'd for branch, don't try delete push config
 . branch, checkout: introduce autosetuppush
 . move deletion of merge configuration to branch.c
 . remote: add per-remote autosetupmerge and autosetuprebase configuration
 . introduce a struct tracking_config
 . branch: install_branch_config and struct tracking refactoring
 . config: allow false and true values for branch.autosetuprebase

Has been ejected from 'pu' for some time, expecting a reroll.

* ne/rev-cache (2009-08-21) 6 commits
 . support for path name caching in rev-cache
 . full integration of rev-cache into git, completed test suite
 . administrative functions for rev-cache, start of integration into git
 . support for non-commit object caching in rev-cache
 . basic revision cache system, no integration or features
 . man page and technical discussion for rev-cache

Updated but seems to break upload-pack tests when merged to 'pu'; given
what this series touches, breakages in that area are expected.
May discard if a working reroll comes, to give it a fresh start.

Re: What's cooking in git.git (Aug 2009, #06; Sun, 30)

[Johan Herland]

On Monday 31 August 2009, Junio C Hamano wrote:
> [Stalled]
>
> * jh/notes (2009-08-27) 12 commits.
>  - Add '%N'-format for pretty-printing commit notes
>  - Add flags to get_commit_notes() to control the format of the note
> string - notes.c: Implement simple memory pooling of leaf nodes
>  - Selftests verifying semantics when loading notes trees with various
> fanouts - Teach the notes lookup code to parse notes trees with various
> fanout schemes - t3302-notes-index-expensive: Speed up create_repo()
>  - fast-import: Add support for importing commit notes
>  - Teach "-m <msg>" and "-F <file>" to "git notes edit"
>  - Add an expensive test for git-notes
>  - Speed up git notes lookup
>  - Add a script to edit/inspect notes
>  - Introduce commit notes
>
> I heard the cvs-helper series depends on this one.  It seems that the
> fan-out strategy is being rethought?

Yes, I'm experimenting with various mixes of date-based and commit_sha1-
based fanouts. Will send a new series when I have some results to show. 
Might not have time to finish before next weekend, though.


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

stash --dwim safety (was Re: What's cooking in git.git (Aug 2009, #06; Sun, 30))

[Junio C Hamano]

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

> [Stalled]
>
> * js/stash-dwim (2009-07-27) 1 commit.
> * tr/reset-checkout-patch (2009-08-27) 9 commits.
>
> There was a discussion on better DWIMmery for the above two topics to (1)
> forbid "git stash save --anything-with-dash" and (2) redirect with any
> option "git stash --opt" to "git stash save --opt", to keep it flexible
> and safe at the same time.  I think it is a sane thing to do, but nothing
> has happened lately.

Actually, I was at fault giving up on Matthieu's patch without studying it
after seeing the phrase "this series replaces", when the two topics the
series tried to replace were already in 'next'.

It turns out that the rework was simple enough, so I did it myself.  Among
his 3 patch series, an equivalent to the first one ("save -keep" can be
written as "save -k" for brevity) were already in, and the second one
(default to "save" if we see any option before command word) was unsafe
without the third one (reject unknown option to "save"), so it ended up as
a single patch that is a combination of the latter two patches.

This applies on top of tr/reset-checkout-patch branch, 14c674e (Make test
case number unique, 2009-08-27).

-- >8 --
From: Matthieu Moy <Matthieu.Moy@imag.fr>
Date: Tue, 18 Aug 2009 23:38:40 +0200
Subject: [PATCH] stash: simplify defaulting to "save" and reject unknown options

With the earlier DWIM patches, certain combination of options defaulted
to the "save" command correctly while certain equally valid combination
did not.  For example, "git stash -k" were Ok but "git stash -q -k" did
not work.

This makes the logic of defaulting to "save" much simpler. If the first
argument begins with a '-', it is clear that there is no command word,
and we default to "save" subcommand.

This also teaches "git stash save" to reject an unknown option.  This is
to keep a mistyped "git stash save --quite" from creating a stash with a
message "--quite", and this safety is more important with the new logic
to default to "save" with any option-looking argument without an explicit
comand word.

[jc: this is based on Matthieu's 3-patch series, and he takes all the
credit; if I have introduced bugs while reworking they are mine]

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

---
 Documentation/git-stash.txt |    1 -
 git-stash.sh                |   22 ++++++++++++++++++----
 t/t3903-stash.sh            |   11 +++++++++++
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 1c4ed41..5d4cce3 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -14,7 +14,6 @@ SYNOPSIS
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
 'git stash' [save [--patch] [-k|--[no-]keep-index] [-q|--quiet] [<message>]]
-'git stash' [-p|--patch|-k|--keep-index]
 'git stash' clear
 'git stash' create
 
diff --git a/git-stash.sh b/git-stash.sh
index 9fd7289..ff71507 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -8,7 +8,6 @@ USAGE="list [<options>]
    or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
    or: $dashless branch <branchname> [<stash>]
    or: $dashless [save [-k|--keep-index] [-q|--quiet] [<message>]]
-   or: $dashless [-k|--keep-index]
    or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
@@ -146,6 +145,14 @@ save_stash () {
 		-q|--quiet)
 			GIT_QUIET=t
 			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			echo "error: unknown option for 'stash save': $1"
+			usage
+			;;
 		*)
 			break
 			;;
@@ -355,6 +362,13 @@ apply_to_branch () {
 	drop_stash $stash
 }
 
+# The default command is "save"
+case "$1" in
+-*)
+	set "save" "$@"
+	;;
+esac
+
 # Main command set
 case "$1" in
 list)
@@ -406,9 +420,9 @@ branch)
 	apply_to_branch "$@"
 	;;
 *)
-	case $#,"$1","$2" in
-	0,,|1,-k,|1,--keep-index,|1,-p,|1,--patch,|2,-p,--no-keep-index|2,--patch,--no-keep-index)
-		save_stash "$@" &&
+	case $# in
+	0)
+		save_stash &&
 		say '(To restore them type "git stash apply")'
 		;;
 	*)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index e16ad93..5514f74 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -208,4 +208,15 @@ test_expect_success 'stash -k' '
 	test bar,bar4 = $(cat file),$(cat file2)
 '
 
+test_expect_success 'stash --invalid-option' '
+	echo bar5 > file &&
+	echo bar6 > file2 &&
+	git add file2 &&
+	test_must_fail git stash --invalid-option &&
+	test_must_fail git stash save --invalid-option &&
+	test bar5,bar6 = $(cat file),$(cat file2) &&
+	git stash -- -message-starting-with-dash &&
+	test bar,bar2 = $(cat file),$(cat file2)
+'
+
 test_done
-- 
1.6.4.2.295.g9bcb

Re: stash --dwim safety

[Matthieu Moy]

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

> It turns out that the rework was simple enough, so I did it myself.  Among
> his 3 patch series, an equivalent to the first one ("save -keep" can be
> written as "save -k" for brevity) were already in, and the second one
> (default to "save" if we see any option before command word) was unsafe
> without the third one (reject unknown option to "save"), so it ended up as
> a single patch that is a combination of the latter two patches.

Thanks, lack of time on my side to work on this, sorry.

I was actually thinking of being a little more paranoid to prevent
accidental "stash save": we could refuse to create a named stash when
the "save" command is not given. The case I hadn't thought of was "git
stash -q apply", which has 99% chances of being a typo for "git stash
apply -q", and which would mean "create a stash named apply, quietly".

> +# The default command is "save"
> +case "$1" in
> +-*)
> +	set "save" "$@"
> +	;;
> +esac

So, that could become something like

default_to_save=t
for arg in "$@"; do
	case "$arg" in
	-*)
		;;
	*)
		default_to_save=
	esac
done

if [ "$default_to_save" = t ]; then
	set "save" "$@"
fi

(untested)

-- 
Matthieu

Re: stash --dwim safety

[Jeff King]

On Tue, Sep 01, 2009 at 08:27:20AM +0200, Matthieu Moy wrote:

> I was actually thinking of being a little more paranoid to prevent
> accidental "stash save": we could refuse to create a named stash when
> the "save" command is not given. The case I hadn't thought of was "git
> stash -q apply", which has 99% chances of being a typo for "git stash
> apply -q", and which would mean "create a stash named apply, quietly".

I like that. I think it addresses Dscho's concern with mistakes causing
an unexpected stash, and it is actually more consistent with the current
rule (that named stashes need an explicit 'save'). IOW, it is actually a
bit confusing that "git stash foo" doesn't work, but "git stash -k foo"
does.

-Peff

Re: What's cooking in git.git (Aug 2009, #06; Sun, 30)

[Peter Krefting]

Junio C Hamano:

> * pk/import-dirs (2009-08-24) 1 commit
> - Add script for importing bits-and-pieces to Git.
>
> This version makes me suspect that the author might regret the choice of 
> the import format that does not allow escaping of paths, nor does not 
> allow leading blanks for readability without changing semantics, both of 
> which make it somewhat limiting and error prone.  These issues will be 
> hard to rectify without breaking the backward compatibility, for a tool 
> that could otherwise turn out to be useful.

If anyone has suggestions on improvements that can help with these issues, 
feel free to submit additional patches. Backwards compatibility is not an 
issue at the moment since it is new material and I so far is the only user 
of the tool (and since it is meant for one-shot imports, backwards 
compatibility is not very important).

-- 
\\// Peter - http://www.softwolves.pp.se/

Re: What's cooking in git.git (Aug 2009, #06; Sun, 30)

[Jakub Narebski]

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

> * jn/gitweb-blame (2009-08-06) 3 commits
>  - gitweb: Create links leading to 'blame_incremental' using JavaScript
>  - gitweb: Incremental blame (WIP)
>  - gitweb: Add optional "time to generate page" info in footer
> 
> Ajax-y blame WIP

There is replacement series sent to git mailing list a little while
ago.  

The replacements for "time to generate page" and 'blame_incremental'
are IMVHO out of WIP (but more testing, in different web browsers
would be good).

The part that actualy creates links that lead to 'blame_incremental'
view is still work in progress, and needs ideas how to correctly
implement it.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: What's cooking in git.git (Aug 2009, #06; Sun, 30)

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> There is replacement series sent to git mailing list a little while
> ago.  

Thanks; I've replaced and pushed them out on 'pu' for now.  Will hopefully
start merging earlier parts to 'next', but how widely is Hires available?

Re: What's cooking in git.git (Aug 2009, #06; Sun, 30)

[Nick Edelen]

> * ne/rev-cache (2009-08-21) 6 commits
>  . support for path name caching in rev-cache
>  . full integration of rev-cache into git, completed test suite
>  . administrative functions for rev-cache, start of integration into git
>  . support for non-commit object caching in rev-cache
>  . basic revision cache system, no integration or features
>  . man page and technical discussion for rev-cache
>
> Updated but seems to break upload-pack tests when merged to 'pu'; given
> what this series touches, breakages in that area are expected.
> May discard if a working reroll comes, to give it a fresh start.

I vaguely remember something concerning those tests when starting the
project.  I'm a bit disconnected from everything right now, but I'll
try to get those fixed as soon as I can.

Re: stash --dwim safety

[Junio C Hamano]

From: Matthieu Moy <Matthieu.Moy@imag.fr>
Date: Tue, 18 Aug 2009 23:38:40 +0200
Subject: [PATCH] stash: simplify defaulting to "save" and reject unknown options

With the earlier DWIM patches, certain combination of options defaulted
to the "save" command correctly while certain equally valid combination
did not.  For example, "git stash -k" were Ok but "git stash -q -k" did
not work.

This makes the logic of defaulting to "save" much simpler. If there is no
non-flag arguments, it is clear that there is no command word, and we
default to "save" subcommand.  This rule prevents "git stash -q apply"
from quietly creating a stash with "apply" as the message.

This also teaches "git stash save" to reject an unknown option.  This is
to keep a mistyped "git stash save --quite" from creating a stash with a
message "--quite", and this safety is more important with the new logic
to default to "save" with any option-looking argument without an explicit
comand word.

[jc: this is based on Matthieu's 3-patch series, and a follow-up
discussion, and he and Peff take all the credit; if I have introduced bugs
while reworking, they are mine.]

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Jeff King <peff@peff.net> writes:

 > On Tue, Sep 01, 2009 at 08:27:20AM +0200, Matthieu Moy wrote:
 >
 >> I was actually thinking of being a little more paranoid to prevent
 >> accidental "stash save": we could refuse to create a named stash when
 >> the "save" command is not given. The case I hadn't thought of was "git
 >> stash -q apply", which has 99% chances of being a typo for "git stash
 >> apply -q", and which would mean "create a stash named apply, quietly".
 >
 > I like that. I think it addresses Dscho's concern with mistakes causing
 > an unexpected stash, and it is actually more consistent with the current
 > rule (that named stashes need an explicit 'save'). IOW, it is actually a
 > bit confusing that "git stash foo" doesn't work, but "git stash -k foo"
 > does.

 Ok, then here comes the final proposal.

 Documentation/git-stash.txt |    9 +++++----
 git-stash.sh                |   27 +++++++++++++++++++++++----
 t/t3903-stash.sh            |   11 +++++++++++
 3 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 1c4ed41..885bc97 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -14,7 +14,6 @@ SYNOPSIS
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
 'git stash' [save [--patch] [-k|--[no-]keep-index] [-q|--quiet] [<message>]]
-'git stash' [-p|--patch|-k|--keep-index]
 'git stash' clear
 'git stash' create
 
@@ -46,9 +45,11 @@ OPTIONS
 save [--patch] [--[no-]keep-index] [-q|--quiet] [<message>]::
 
 	Save your local modifications to a new 'stash', and run `git reset
-	--hard` to revert them.  This is the default action when no
-	subcommand is given. The <message> part is optional and gives
-	the description along with the stashed state.
+	--hard` to revert them.  The <message> part is optional and gives
+	the description along with the stashed state.  For quickly making
+	a snapshot, you can omit _both_ "save" and <message>, but giving
+	only <message> does not trigger this action to prevent misspelled
+	subcommand from making an unwanted stash.
 +
 If the `--keep-index` option is used, all changes already added to the
 index are left intact.
diff --git a/git-stash.sh b/git-stash.sh
index 9fd7289..f243376 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -8,7 +8,6 @@ USAGE="list [<options>]
    or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
    or: $dashless branch <branchname> [<stash>]
    or: $dashless [save [-k|--keep-index] [-q|--quiet] [<message>]]
-   or: $dashless [-k|--keep-index]
    or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
@@ -146,6 +145,14 @@ save_stash () {
 		-q|--quiet)
 			GIT_QUIET=t
 			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			echo "error: unknown option for 'stash save': $1"
+			usage
+			;;
 		*)
 			break
 			;;
@@ -355,6 +362,18 @@ apply_to_branch () {
 	drop_stash $stash
 }
 
+# The default command is "save" if nothing but options are given
+seen_non_option=
+for opt
+do
+	case "$opt" in
+	-*) ;;
+	*) seen_non_option=t; break ;;
+	esac
+done
+
+test -n "$seen_non_option" || set "save" "$@"
+
 # Main command set
 case "$1" in
 list)
@@ -406,9 +425,9 @@ branch)
 	apply_to_branch "$@"
 	;;
 *)
-	case $#,"$1","$2" in
-	0,,|1,-k,|1,--keep-index,|1,-p,|1,--patch,|2,-p,--no-keep-index|2,--patch,--no-keep-index)
-		save_stash "$@" &&
+	case $# in
+	0)
+		save_stash &&
 		say '(To restore them type "git stash apply")'
 		;;
 	*)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index e16ad93..5514f74 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -208,4 +208,15 @@ test_expect_success 'stash -k' '
 	test bar,bar4 = $(cat file),$(cat file2)
 '
 
+test_expect_success 'stash --invalid-option' '
+	echo bar5 > file &&
+	echo bar6 > file2 &&
+	git add file2 &&
+	test_must_fail git stash --invalid-option &&
+	test_must_fail git stash save --invalid-option &&
+	test bar5,bar6 = $(cat file),$(cat file2) &&
+	git stash -- -message-starting-with-dash &&
+	test bar,bar2 = $(cat file),$(cat file2)
+'
+
 test_done
-- 
1.6.4.2.301.g12b4ad

Re: stash --dwim safety

[Jeff King]

[cc'ing Dscho, as he was the main opponent of similar proposals, and I
suspect his silence here means he missed this discussion. I hope this
addresses his concerns, but I think it is good to get comment from all
interested parties.

I'll just quote as appropriate below to comment, but for the whole patch
see:

  http://article.gmane.org/gmane.comp.version-control.git/127574

]

On Tue, Sep 01, 2009 at 09:11:37PM -0700, Junio C Hamano wrote:

> This makes the logic of defaulting to "save" much simpler. If there is no
> non-flag arguments, it is clear that there is no command word, and we

s/is/are/ (or s/arguments/argument/)

> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> [...]
> -	--hard` to revert them.  This is the default action when no
> -	subcommand is given. The <message> part is optional and gives
> -	the description along with the stashed state.
> +	--hard` to revert them.  The <message> part is optional and gives
> +	the description along with the stashed state.  For quickly making
> +	a snapshot, you can omit _both_ "save" and <message>, but giving
> +	only <message> does not trigger this action to prevent misspelled
> +	subcommand from making an unwanted stash.

s/misspelled/a &/

-Peff

Re: What's cooking in git.git (Aug 2009, #06; Sun, 30)

[Junio C Hamano]

Nick Edelen <sirnot@gmail.com> writes:

> I vaguely remember something concerning those tests when starting the
> project.  I'm a bit disconnected from everything right now, but I'll
> try to get those fixed as soon as I can.

Thanks.

Re: stash --dwim safety

[Matthieu Moy]

Thanks for taking care of this,

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

>  'git stash' [save [--patch] [-k|--[no-]keep-index] [-q|--quiet] [<message>]]

To be precise, you can change this to

>  'git stash' [save [--patch] [-k|--[no-]keep-index] [-q|--quiet] [--] [<message>]]

(added [--])

-- 
Matthieu

Re: stash --dwim safety

[Johannes Schindelin]

Hi,

On Wed, 2 Sep 2009, Jeff King wrote:

> [cc'ing Dscho, as he was the main opponent of similar proposals, and I
> suspect his silence here means he missed this discussion.

Your assumption is correct.

I am overloaded with work, and in such times it is highly unlikely that I 
get back to a discussion that was less than fun.

Ciao,
Dscho

Re: What's cooking in git.git (Aug 2009, #06; Sun, 30)

[Jakub Narebski]

On Tue, 1 Sep 2009, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > There is replacement series sent to git mailing list a little while
> > ago.  
> 
> Thanks; I've replaced and pushed them out on 'pu' for now.  Will hopefully
> start merging earlier parts to 'next', but how widely is Hires available?

Well, if someone wants to have _optional_ 'timed' feature, ha/she can
install Time::HiRes module.  I think that it is not in Perl core, but
there are RPM and deb packages with Time::HiRes available in extras.
If module is not installed, then only 'timed' feature is not available.


P.S. "Naming is the hardest thing"; should this feature be named 'timed',
or do any of you have some better name for it?

P.P.S. Originally the part about "time to generate page" was for me to be
able to benchmark new code... but then I realized that benchmarking 
'blame_incremental' view on single-core computer, where server process
and AJAX-y JavaScript competes for CPU doesn't a good benchmark make.
Still, this part can be useful.

-- 
Jakub Narebski
Poland

Re: What's cooking in git.git (Aug 2009, #06; Sun, 30)

[Peter Harris]

On Tue, Sep 1, 2009 at 12:51 PM, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
>> There is replacement series sent to git mailing list a little while
>> ago.
>
> Thanks; I've replaced and pushed them out on 'pu' for now.  Will hopefully
> start merging earlier parts to 'next', but how widely is Hires available?

It was added to the Perl core in 5.8. Gitweb already depends on 5.8,
according to http://article.gmane.org/gmane.comp.version-control.git/83339

Peter Harris