What's cooking in git.git (Feb 2011, #06; Sun, 27)

What's cooking in git.git (Feb 2011, #06; Sun, 27)

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

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

* en/object-list-with-pathspec (2010-09-20) 2 commits
  (merged to 'next' on 2011-02-09 at ccf6c6a)
 + Add testcases showing how pathspecs are handled with rev-list --objects
 + Make rev-list --objects work together with pathspecs
 (this branch uses nd/struct-pathspec; is tangled with jc/grep--no-index-pathspec-fix.)

* hv/mingw-fs-funnies (2011-02-07) 5 commits
  (merged to 'next' on 2011-02-09 at 3d0bb1a)
 + mingw_rmdir: set errno=ENOTEMPTY when appropriate
 + mingw: add fallback for rmdir in case directory is in use
 + mingw: make failures to unlink or move raise a question
 + mingw: work around irregular failures of unlink on windows
 + mingw: move unlink wrapper to mingw.c

* jh/push-default-upstream-configname (2011-02-16) 1 commit
  (merged to 'next' on 2011-02-23 at b5c25fa)
 + push.default: Rename 'tracking' to 'upstream'

* js/detach-doc (2011-02-20) 1 commit
  (merged to 'next' on 2011-02-21 at c384c3c)
 + git-checkout.txt: improve detached HEAD documentation

* js/maint-merge-use-prepare-commit-msg-hook (2011-02-14) 1 commit
  (merged to 'next' on 2011-02-22 at 6458c4b)
 + merge: honor prepare-commit-msg hook

* lp/config-vername-check (2011-02-01) 2 commits
  (merged to 'next' on 2011-02-23 at 426d48d)
 + Disallow empty section and variable names
 + Sanity-check config variable names

* mg/patch-id (2011-02-17) 2 commits
  (merged to 'next' on 2011-02-22 at 6f4acd8)
 + git-patch-id: do not trip over "no newline" markers
 + git-patch-id: test for "no newline" markers

* mg/placeholders-are-lowercase (2011-02-17) 5 commits
  (merged to 'next' on 2011-02-22 at 2754e21)
 + Make <identifier> lowercase in Documentation
 + Make <identifier> lowercase as per CodingGuidelines
 + Make <identifier> lowercase as per CodingGuidelines
 + Make <identifier> lowercase as per CodingGuidelines
 + CodingGuidelines: downcase placeholders in usage messages

* mo/perl-bidi-pipe-envfix (2011-02-15) 1 commit
  (merged to 'next' on 2011-02-15 at c36e816)
 + perl: command_bidi_pipe() method should set-up git environmens

* mz/rerere-remaining (2011-02-16) 2 commits
  (merged to 'next' on 2011-02-22 at fa2d5ab)
 + mergetool: don't skip modify/remove conflicts
 + rerere "remaining"

* nd/hash-object-sanity (2011-02-05) 1 commit
  (merged to 'next' on 2011-02-22 at 09acf6f)
 + Make hash-object more robust against malformed objects

* nd/sorted-builtin-command-list (2011-02-15) 1 commit
  (merged to 'next' on 2011-02-22 at 91fccd1)
 + git.c: reorder builtin command list

* nd/struct-pathspec (2011-01-31) 22 commits
  (merged to 'next' on 2011-02-09 at b1e64ee)
 + t6004: add pathspec globbing test for log family
 + t7810: overlapping pathspecs and depth limit
 + grep: drop pathspec_matches() in favor of tree_entry_interesting()
 + grep: use writable strbuf from caller for grep_tree()
 + grep: use match_pathspec_depth() for cache/worktree grepping
 + grep: convert to use struct pathspec
 + Convert ce_path_match() to use match_pathspec_depth()
 + Convert ce_path_match() to use struct pathspec
 + struct rev_info: convert prune_data to struct pathspec
 + pathspec: add match_pathspec_depth()
 + tree_entry_interesting(): optimize wildcard matching when base is matched
 + tree_entry_interesting(): support wildcard matching
 + tree_entry_interesting(): fix depth limit with overlapping pathspecs
 + tree_entry_interesting(): support depth limit
 + tree_entry_interesting(): refactor into separate smaller functions
 + diff-tree: convert base+baselen to writable strbuf
 + glossary: define pathspec
 + Move tree_entry_interesting() to tree-walk.c and export it
 + tree_entry_interesting(): remove dependency on struct diff_options
 + Convert struct diff_options to use struct pathspec
 + diff-no-index: use diff_tree_setup_paths()
 + Add struct pathspec
 (this branch is used by en/object-list-with-pathspec and jc/grep--no-index-pathspec-fix.)

* pw/p4 (2011-02-19) 8 commits
  (merged to 'next' on 2011-02-21 at 1a7b7d2)
 + git-p4: support clone --bare
 + git-p4: decode p4 wildcard characters
 + git-p4: better message for "git-p4 sync" when not cloned
 + git-p4: reinterpret confusing p4 message
 + git-p4: accommodate new move/delete type in p4
 + git-p4: add missing newline in initial import message
 + git-p4: fix key error for p4 problem
 + git-p4: test script

* sp/maint-smart-http-sans-100-continue (2011-02-15) 1 commit
  (merged to 'next' on 2011-02-15 at 553e3e5)
 + smart-http: Don't use Expect: 100-Continue

* uk/checkout-ambiguous-ref (2011-02-15) 5 commits
  (merged to 'next' on 2011-02-15 at 645dad6)
 + Rename t2019 with typo "amiguous" that meant "ambiguous"
 + checkout: rearrange update_refs_for_switch for clarity
 + checkout: introduce --detach synonym for "git checkout foo^{commit}"
 + checkout: split off a function to peel away branchname arg
  (merged to 'next' on 2011-02-03 at 9044724)
 + checkout: fix bug with ambiguous refs

* va/p4 (2011-02-20) 2 commits
  (merged to 'next' on 2011-02-21 at d981b23)
 + git-p4: Add copy detection support
 + git-p4: Improve rename detection support

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

* jn/maint-commit-missing-template (2011-02-25) 1 commit
  (merged to 'next' on 2011-02-25 at c95589d)
 + commit: error out for missing commit message template

* mg/maint-difftool-vim-readonly (2011-02-25) 1 commit
  (merged to 'next' on 2011-02-25 at 990579c)
 + mergetool-lib: call vim in readonly mode for diffs

* fk/maint-cvsimport-early-failure (2011-01-31) 1 commit
 - git-cvsimport.perl: Bail out right away when reading from the server fails

* jk/strbuf-vaddf (2011-02-25) 2 commits
 - strbuf: add strbuf_vaddf
 - compat: provide a fallback va_copy definition
 (this branch is used by ab/i18n-st, jk/trace-sifter and jn/status-translatable.)

* jk/trace-sifter (2011-02-24) 6 commits
 - trace: give repo_setup trace its own key
 - add packet tracing debug code
 - trace: add trace_strbuf
 - trace: factor out "do we want to trace" logic
 - trace: refactor to support multiple env variables
 - trace: add trace_vprintf
 (this branch uses jk/strbuf-vaddf; is tangled with ab/i18n-st and jn/status-translatable.)

* jn/maint-instaweb-plack-fix (2011-02-26) 1 commit
 - git-instaweb: Change how gitweb.psgi is made runnable as standalone app

* jn/status-translatable (2011-02-25) 3 commits
 - commit, status: use status_printf{,_ln,_more} helpers
 - commit: refer to commit template as s->fp
 - wt-status: add helpers for printing wt-status lines
 (this branch is used by ab/i18n-st and ab/i18n-st; uses jk/strbuf-vaddf; is tangled with jk/trace-sifter.)

* mh/p4 (2011-02-25) 1 commit
  (merged to 'next' on 2011-02-26 at 1693331)
 + git-p4 submit: prevent 'Jobs' section from being removed from p4 change log

* nd/rfc-add-u-full-tree (2011-02-07) 1 commit
 - add: make "add -u" update full tree without pathspec

* ss/git-gui-mergetool (2011-02-26) 2 commits
 - mergetool--lib: Add Beyond Compare 3 as a tool
 - mergetool--lib: Sort tools alphabetically for easier lookup

* ss/mergetool--lib (2011-02-27) 2 commits
 - mergetool--lib: Add Beyond Compare 3 as a tool
 - mergetool--lib: Sort tools alphabetically for easier lookup

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

* jh/merge-sans-branch (2011-02-10) 4 commits
 . merge: add support for merging from upstream by default
 - merge: introduce per-branch-configuration helper function
 - merge: introduce setup_merge_commit helper function
 - merge: update the usage information to be more modern

There was an objection to the tip one that determines the upstream in a
wrong way?

* jk/tag-contains (2010-07-05) 4 commits
 - Why is "git tag --contains" so slow?
 - default core.clockskew variable to one day
 - limit "contains" traversals based on commit timestamp
 - tag: speed up --contains calculation

The idea of the bottom one is probably Ok, except that the use of object
flags needs to be rethought, or at least the helper needs to be moved to
builtin/tag.c to make it clear that it should not be used outside the
current usage context.

* jc/rename-degrade-cc-to-c (2011-01-06) 3 commits
 . diffcore-rename: fall back to -C when -C -C busts the rename limit
 . diffcore-rename: record filepair for rename src
 . diffcore-rename: refactor "too many candidates" logic

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

* nd/index-doc (2010-09-06) 1 commit
 - doc: technical details about the index file format

* ab/i18n-st (2011-02-22) 74 commits
 - i18n: git-shortlog basic messages
 - i18n: git-revert split up "could not revert/apply" message
 - i18n: git-revert literal "me" messages
 - i18n: git-revert "Your local changes" message
 - i18n: git-revert basic messages
 - i18n: git-notes GIT_NOTES_REWRITE_MODE error message
 - i18n: git-notes basic commands
 - i18n: git-gc "Auto packing the repository" message
 - i18n: git-gc basic messages
 - i18n: git-describe basic messages
 - i18n: git-clean clean.requireForce messages
 - i18n: git-clean basic messages
 - i18n: git-bundle basic messages
 - i18n: git-archive basic messages
 - i18n: git-status "renamed: " message
 - i18n: git-status "Initial commit" message
 - i18n: git-status "Changes to be committed" message
 - i18n: git-status shortstatus messages
 - i18n: git-status "nothing to commit" messages
 - i18n: git-status basic messages
 - i18n: git-push "prevent you from losing" message
 - i18n: git-push basic messages
 - i18n: git-tag tag_template message
 - i18n: git-tag basic messages
 - i18n: git-reset "Unstaged changes after reset" message
 - i18n: git-reset reset_type_names messages
 - i18n: git-reset basic messages
 - i18n: git-rm basic messages
 - i18n: git-mv "bad" messages
 - i18n: git-mv basic messages
 - i18n: git-merge "Wonderful" message
 - i18n: git-merge "You have not concluded your merge" messages
 - i18n: git-merge "Updating %s..%s" message
 - i18n: git-merge basic messages
 - i18n: git-log "--OPT does not make sense" messages
 - i18n: git-log basic messages
 - i18n: git-grep "--open-files-in-pager" message
 - i18n: git-grep basic messages
 - i18n: git-fetch split up "(non-fast-forward)" message
 - i18n: git-fetch update_local_ref messages
 - i18n: git-fetch formatting messages
 - i18n: git-fetch basic messages
 - i18n: git-diff basic messages
 - i18n: git-commit advice messages
 - i18n: git-commit "enter the commit message" message
 - i18n: git-commit print_summary messages
 - i18n: git-commit formatting messages
 - i18n: git-commit "middle of a merge" message
 - i18n: git-commit basic messages
 - i18n: git-checkout "Switched to a .. branch" message
 - i18n: git-checkout "HEAD is now at" message
 - i18n: git-checkout describe_detached_head messages
 - i18n: git-checkout: our/their version message
 - i18n: git-checkout basic messages
 - i18n: git-branch "(no branch)" message
 - i18n: git-branch "git branch -v" messages
 - i18n: git-branch "Deleted branch [...]" message
 - i18n: git-branch "remote branch '%s' not found" message
 - i18n: git-branch basic messages
 - i18n: git-add "Unstaged changes" message
 - i18n: git-add "remove '%s'" message
 - i18n: git-add "did not match any files" message
 - i18n: git-add "The following paths are ignored" message
 - i18n: git-add basic messages
 - i18n: git-clone "Cloning into" message
 - i18n: git-clone "Cloning into" message
 - i18n: git-clone basic messages
 - i18n: git-init "Initialized [...] repository" message
 - i18n: git-init basic messages
 - i18n: "make distclean" should clean up after "make pot"
 - i18n: Makefile: "pot" target to extract messages marked for translation
 - i18n: do not poison translations unless GIT_GETTEXT_POISON envvar is set
 - i18n: add GETTEXT_POISON to simulate unfriendly translator
 - i18n: add no-op _() and N_() wrappers
 (this branch uses jk/strbuf-vaddf, jn/status-translatable and jn/status-translatable; is tangled with jk/trace-sifter.)

Rebased on other infrastructure adjustments (tentatively renamed the
branch).  I'd like to fast-track the basics (especially the bottom 3
patches), and am even tempted to rebase other patches on 'pu' that are not
yet in 'next' on top of them, to make the transition easier.

* gr/cvsimport-alternative-cvspass-location (2011-02-18) 1 commit
 - Look for password in both CVS and CVSNT password files.

* jc/checkout-orphan-warning (2011-02-18) 1 commit
 - commit: give final warning when reattaching HEAD to leave commits behind

Likes, dislikes?

* jh/maint-do-not-track-non-branches (2011-02-17) 1 commit
 - branch/checkout --track: Ensure that upstream branch is indeed a branch

* jk/diffstat-binary (2011-02-19) 2 commits
  (merged to 'next' on 2011-02-23 at 49da967)
 + diff: don't retrieve binary blobs for diffstat
 + diff: handle diffstat of rewritten binary files

* jk/fail-null-clone (2011-02-17) 1 commit
  (merged to 'next' on 2011-02-23 at a4217f5)
 + clone: die when trying to clone missing local path

* jk/merge-rename-ux (2011-02-20) 6 commits
 - pull: propagate --progress to merge
 - merge: enable progress reporting for rename detection
 - add inexact rename detection progress infrastructure
 - commit: stop setting rename limit
 - bump rename limit defaults (again)
 - merge: improve inexact rename limit warning

The above three all seemed sensible improvements.

* jn/test-terminal-punt-on-osx-breakage (2011-02-17) 1 commit
  (merged to 'next' on 2011-02-23 at d754139)
 + tests: skip terminal output tests on OS X

* js/cherry-pick-usability (2011-02-19) 4 commits
  (merged to 'next' on 2011-02-23 at 95db30e)
 + Teach commit about CHERRY_PICK_HEAD
 + bash: teach __git_ps1 about CHERRY_PICK_HEAD
 + Introduce CHERRY_PICK_HEAD
 + t3507: introduce pristine-detach helper

* lt/rename-no-extra-copy-detection (2011-02-18) 3 commits
  (merged to 'next' on 2011-02-23 at 2c1f271)
 + diffcore-rename: improve estimate_similarity() heuristics
 + diffcore-rename: properly honor the difference between -M and -C
 + for_each_hash: allow passing a 'void *data' pointer to callback

* mg/rev-list-one-side-only (2011-02-22) 6 commits
 - t6007: test rev-list --cherry
 - log --cherry: a synonym
 - rev-list: --left/right-only are mutually exclusive
 - rev-list: documentation and test for --left/right-only
 - t6007: Make sure we test --cherry-pick
 - revlist.c: introduce --left/right-only for unsymmetric picking

* so/submodule-no-update-first-time (2011-02-17) 2 commits
  (merged to 'next' on 2011-02-23 at 2c6e8c9)
 + t7406: "git submodule update {--merge|--rebase]" with new submodules
 + submodule: no [--merge|--rebase] when newly cloned

* jc/complete-symmetric-diff (2011-02-23) 1 commit
 - completion: complete "git diff ...branc<TAB>"

* jh/submodule-fetch-on-demand (2011-02-23) 6 commits
 - submodule update: Don't fetch when the submodule commit is already present
 - fetch/pull: Don't recurse into a submodule when commits are already present
 - Submodules: Add 'on-demand' value for the 'fetchRecurseSubmodule' option
 - config: teach the fetch.recurseSubmodules option the 'on-demand' value
 - fetch/pull: Add the 'on-demand' value to the --recurse-submodules option
 - fetch/pull: recurse into submodules when necessary

* jk/format-patch-multiline-header (2011-02-23) 3 commits
 - format-patch: rfc2047-encode newlines in headers
 - format-patch: wrap long header lines
 - strbuf: add fixed-length version of add_wrapped_text

* js/checkout-untracked-symlink (2011-02-20) 2 commits
  (merged to 'next' on 2011-02-23 at 52a35ce)
 + do not overwrite untracked symlinks
 + Demonstrate breakage: checkout overwrites untracked symlink with directory

* jc/grep--no-index-pathspec-fix (2011-02-16) 1 commit
  (merged to 'next' on 2011-02-23 at 58b03b1)
 + grep --no-index: honor pathspecs correctly

* mz/rebase (2011-02-24) 33 commits
  (merged to 'next' on 2011-02-25 at 52caa7a)
 + Makefile: do not install sourced rebase scripts
  (merged to 'next' on 2011-02-22 at 3219155)
 + rebase: use @{upstream} if no upstream specified
 + rebase -i: remove unnecessary state rebase-root
 + rebase -i: don't read unused variable preserve_merges
 + git-rebase--am: remove unnecessary --3way option
 + rebase -m: don't print exit code 2 when merge fails
 + rebase -m: remember allow_rerere_autoupdate option
 + rebase: remember strategy and strategy options
 + rebase: remember verbose option
 + rebase: extract code for writing basic state
 + rebase: factor out sub command handling
 + rebase: make -v a tiny bit more verbose
 + rebase -i: align variable names
 + rebase: show consistent conflict resolution hint
 + rebase: extract am code to new source file
 + rebase: extract merge code to new source file
 + rebase: remove $branch as synonym for $orig_head
 + rebase -i: support --stat
 + rebase: factor out call to pre-rebase hook
 + rebase: factor out clean work tree check
 + rebase: factor out reference parsing
 + rebase: reorder validation steps
 + rebase -i: remove now unnecessary directory checks
 + rebase: factor out command line option processing
 + rebase: align variable content
 + rebase: align variable names
 + rebase: stricter check of standalone sub command
 + rebase: act on command line outside parsing loop
 + rebase: improve detection of rebase in progress
 + rebase: remove unused rebase state 'prev_head'
 + rebase: read state outside loop
 + rebase: refactor reading of state
 + rebase: clearer names for directory variables

Minor UI regression was reported but otherwise it looked like that the
topic is in a good shape.

--------------------------------------------------
[Discarded]

* cp/mergetool-beyondcompare (2011-02-18) 1 commit
 . mergetool--lib: add support for beyond compare

ss/mergetool--lib replaces this.

Re: What's cooking in git.git (Feb 2011, #06; Sun, 27)

[Sverre Rabbelier]

Heya,

On Mon, Feb 28, 2011 at 07:48, Junio C Hamano <gitster@pobox.com> wrote:
> * jc/checkout-orphan-warning (2011-02-18) 1 commit
>  - commit: give final warning when reattaching HEAD to leave commits behind
>
> Likes, dislikes?

I can't find a message containing this commit title, can you link to
the relevant thread? I did find that in the last what's cooking Ævar
replied and said +1? I agree that this is something we want to do
though, I'd like all operations in git to be as data-preserving as
possible, and to be similarly helpful to users about not losing data.
I think this is a good step in that direction.


-- 
Cheers,

Sverre Rabbelier

Re: What's cooking in git.git (Feb 2011, #06; Sun, 27)

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Heya,
>
> On Mon, Feb 28, 2011 at 07:48, Junio C Hamano <gitster@pobox.com> wrote:
>> * jc/checkout-orphan-warning (2011-02-18) 1 commit
>>  - commit: give final warning when reattaching HEAD to leave commits behind
>>
>> Likes, dislikes?
>
> I can't find a message containing this commit title, can you link to
> the relevant thread?

This is a re-roll of a fallout from this thread:

    http://thread.gmane.org/gmane.comp.version-control.git/166595/focus=167133

I do think the safety net for detached HEAD is a bit too flimsy, and that
is the motivation behind this.

As I don't think it is necessarily a good idea to attempt to make every
possible operation revertible (e.g., I do not think "oops, I did 'git add'
and I have only "git lost-and-found" to find the old index entry" is a
problem worth solving with extra complexity and storage cost), we should
strike a good balance by adding safety only at key places in reasonable
workflows (as opposed to every step to clutter the "undo log").

I am not convinced that this patch nailed that balance at exactly the
right place, even though I think we are getting closer than before.  In
this sequence:

    $ git checkout somewhere^0
    $ git commit
    $ git reset --hard somewhere_else
    $ git commit
    $ git checkout master

The second commit is protected with the patch, but not the first one,
which is still protected by the reflog on HEAD (i.e. "git log -g HEAD").
You have to know the reflog on HEAD if you _intend to_ work on detached
HEAD anyway, and you don't need this patch if you do---the second commit
can also be recovered from the reflog on HEAD.

So this patch is not about helping people who _chose to_ work on detached
HEAD; instead the patch in its current form is about helping sightseers
who has become _only_ a bit adventurous.  To help people who accidentally
started from a detached state (i.e. starting from sightseeing but then got
very adventurous, forgetting that they are not on any branch) and spent
significant amount of time committing and resetting, the advice message
should teach them to use "log -g HEAD" more explicitly, instead of giving
details of the last state (i.e. "you are leaving %n commits behind...").

Re: What's cooking in git.git (Feb 2011, #06; Sun, 27)

[Jeff King]

On Mon, Feb 28, 2011 at 08:52:08AM -0800, Junio C Hamano wrote:

> > On Mon, Feb 28, 2011 at 07:48, Junio C Hamano <gitster@pobox.com> wrote:
> >> * jc/checkout-orphan-warning (2011-02-18) 1 commit
> >>  - commit: give final warning when reattaching HEAD to leave commits behind
> >>
> >> Likes, dislikes?
> >
> > I can't find a message containing this commit title, can you link to
> > the relevant thread?
> 
> This is a re-roll of a fallout from this thread:
> 
>     http://thread.gmane.org/gmane.comp.version-control.git/166595/focus=167133
> 
> I do think the safety net for detached HEAD is a bit too flimsy, and that
> is the motivation behind this.

I've been meaning to look more closely at this for a while. I like it.
There is some small overhead in the revision traversal, but in my
(admittedly not very rigorous) tests it is not noticeable.

I complained before about the possibility of going to the roots, but of
course that doesn't happen here because the regular traversal does
respect commit timestamps as a hint to stop (so it is subject to skew,
but that is nothing new).

> I am not convinced that this patch nailed that balance at exactly the
> right place, even though I think we are getting closer than before.  In
> this sequence:
> 
>     $ git checkout somewhere^0
>     $ git commit
>     $ git reset --hard somewhere_else
>     $ git commit
>     $ git checkout master
> 
> The second commit is protected with the patch, but not the first one,
> which is still protected by the reflog on HEAD (i.e. "git log -g HEAD").
> You have to know the reflog on HEAD if you _intend to_ work on detached
> HEAD anyway, and you don't need this patch if you do---the second commit
> can also be recovered from the reflog on HEAD.

I really wouldn't expect it to help here. The problem isn't that you're
on a detached HEAD. It's that you're using "git reset" at all. If you
did that while on a branch, you would still have to pull the result from
the reflog. Maybe somebody has a clever idea to deal with that, but it
is a separate problem (and personally, I think the answer is "reset is
dangerous; don't use it". That isn't the case with the detached head,
because checkout and commit are usually safe, and it is more about
people being confused about their state when they make commits).

As far as the balance struck in the patch, the decisions I see are:

  1. When to warn. I think "when commit does not match a ref exactly" is
     going to have too many false positives. Reachability seems like the
     best answer, as long as the computation time doesn't turn out to be
     too expensive.

  2. Whether to block the checkout or warn.  You chose warn, and I
     agree. The advice for fixing it is the same whether we go through
     with the checkout or not. Blocking it just makes the "so what, I
     want to throw those away" choice unnecessarily annoying.

So I'm curious what you think is missing in the balance you mentioned
above.

A few other things I noticed are:

  - I like that you show the commits about to be dropped. That helps the
    user make the decision, and it is not expensive to calculate since
    we are in the uncommon "safety valve kicks in" case.

    I did find the " - oneline" format a little off. Maybe just because
    I am so used to regular git output. But I think just using " %h %s",
    e.g.:

       1234abcd commit subject one
       5678defa commit subject two

    would be better. Users are (hopefully) used to seeing commits listed
    in that form, and the sha1s are useful if you are a clueful user and
    your decision isn't to make a branch, but rather to say "Oh, the top
    one is junk but the second is useful". And then you can cherry-pick
    it, look at it in more detail, or whatever.

    In other words, I think this safety valve is not just useful for
    clueless people who make commits without realizing they're on a
    detached HEAD. It is also useful for clueful people as a helpful
    reminder that they may be leaving commits behind, and they can
    ignore or deal with it as appropriate. I tend to ignore the
    "Previous HEAD was..." message because it shows _every time_,
    whether those are my new commits or not.

  - It should probably be wrapped in advice.abandonDetachedCommits or
    something. If turned off, we can omit the check altogether
    (and possibly retain the "Previous HEAD was..." message, though I'm
    not sure). But I can also imagine a "short" version that just shows
    a single-line "warning: you are leaving %d commits behind" and the
    oneline of each, but without the "here's where to go from there"
    advice. I would probably use the short version myself.

  - Nit: you nicely use "%d commit%s" to handle the single/plural case
    in the warning message, but then you "them" later on. It needs
    (1 < lost) ? "them" : "it".

> So this patch is not about helping people who _chose to_ work on detached
> HEAD; instead the patch in its current form is about helping sightseers
> who has become _only_ a bit adventurous.  To help people who accidentally
> started from a detached state (i.e. starting from sightseeing but then got
> very adventurous, forgetting that they are not on any branch) and spent
> significant amount of time committing and resetting, the advice message
> should teach them to use "log -g HEAD" more explicitly, instead of giving
> details of the last state (i.e. "you are leaving %n commits behind...").

Ah, I think this is maybe where your reset example comes in. I remember
in early discussions of detached HEAD you recommending the workflow:

  # sightsee to some tag
  git checkout v1.7.4
  # now sightsee somewhere else
  git reset --hard v1.7.3
  # now go back to a branch
  git checkout master

But isn't that second step much better accomplished with "git checkout
v1.7.3" ? It's conceptually much simpler (you use the same command to
sightsee the first time as the second, and you don't have to know what
in the world --hard means), it's less typing, and it has better safety
valves (even before the valve we are talking about).

So I don't think it is worth thinking too hard about people doing random
resets in the middle of a detached HEAD. We should just not recommend
reset as a command for this (and indeed, I think it already has a bit of
a reputation among newer git users as a dangerous and scary thing to
do. Which is probably fine).

If we do want to teach reset users about reflog, then I think that is a
separate issue from the detached HEAD case (and I would _certainly_ wrap
that in an advice.* config :) ).

-Peff

Re: What's cooking in git.git (Feb 2011, #06; Sun, 27)

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Feb 28, 2011 at 08:52:08AM -0800, Junio C Hamano wrote:
>
>> I am not convinced that this patch nailed that balance at exactly the
>> right place, even though I think we are getting closer than before.  In
>> this sequence:
>> 
>>     $ git checkout somewhere^0
>>     $ git commit
>>     $ git reset --hard somewhere_else
>>     $ git commit
>>     $ git checkout master
>> ...
> I really wouldn't expect it to help here. The problem isn't that you're
> on a detached HEAD. It's that you're using "git reset" at all.

As you would realize later in your message, the "reset --hard" can instead
be "checkout v1.7.3".  And the bothersome thing is that there is no safety
against that.  We don't bother users while they are jumping around on
detached HEAD, and it is not new; we don't say where the previous detached
HEAD was.

Perhaps that needs to be changed to have the same safety?  IOW, regardless
of your "checkout" destination, whenever you leave a detached commit that
would become unreachable, we should warn the same way?

I suspect I would loath it as being too loud; I dunno...

Re: What's cooking in git.git (Feb 2011, #06; Sun, 27)

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> So I'm curious what you think is missing in the balance you mentioned
> above.

I touched this in a separate message, I think...

> A few other things I noticed are:
> ...
>     I am so used to regular git output. But I think just using " %h %s",
>     e.g.:
>
>        1234abcd commit subject one
>        5678defa commit subject two

That would be better, I agree.

> ...
>     In other words, I think this safety valve is not just useful for
>     clueless people who make commits without realizing they're on a
>     detached HEAD. It is also useful for clueful people as a helpful
>     reminder that they may be leaving commits behind, and they can
>     ignore or deal with it as appropriate. I tend to ignore the
>     "Previous HEAD was..." message because it shows _every time_,
>     whether those are my new commits or not.

... never thought about this from that angle.  Perhaps you are right.

>   - Nit: you nicely use "%d commit%s" to handle the single/plural case
>     in the warning message, but then you "them" later on. It needs
>     (1 < lost) ? "them" : "it".

I actually don't like playing games like that, especially when i18n topic
is in flight.  Among the languages I know rules reasonably well, two has
the rule that a countable noun is spelled differently depending on the
number of that thing is one or more, and one spells the noun the same way
regardless of the number.  Who knows if git needs to be translated into a
language whose noun changes its shape three-way, depending on the number
being one, two, or more?

Re: What's cooking in git.git (Feb 2011, #06; Sun, 27)

[Piotr Krukowiecki]

On Wed, Mar 2, 2011 at 6:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>>   - Nit: you nicely use "%d commit%s" to handle the single/plural case
>>     in the warning message, but then you "them" later on. It needs
>>     (1 < lost) ? "them" : "it".
>
> I actually don't like playing games like that, especially when i18n topic
> is in flight.  Among the languages I know rules reasonably well, two has
> the rule that a countable noun is spelled differently depending on the
> number of that thing is one or more, and one spells the noun the same way
> regardless of the number.  Who knows if git needs to be translated into a
> language whose noun changes its shape three-way, depending on the number
> being one, two, or more?

For gettex this is described at
http://www.gnu.org/software/gettext/manual/gettext.html#Plural-forms

Don't know how it's handled in shell scripts or perl or whatever other
language (which does not use gettext?)


-- 
Piotrek

Re: What's cooking in git.git (Feb 2011, #06; Sun, 27)

[Jakub Narebski]

Piotr Krukowiecki <piotr.krukowiecki@gmail.com> writes:
> On Wed, Mar 2, 2011 at 6:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jeff King <peff@peff.net> writes:

>>>   - Nit: you nicely use "%d commit%s" to handle the single/plural case
>>>     in the warning message, but then you "them" later on. It needs
>>>     (1 < lost) ? "them" : "it".
>>
>> I actually don't like playing games like that, especially when i18n topic
>> is in flight.  Among the languages I know rules reasonably well, two has
>> the rule that a countable noun is spelled differently depending on the
>> number of that thing is one or more, and one spells the noun the same way
>> regardless of the number.  Who knows if git needs to be translated into a
>> language whose noun changes its shape three-way, depending on the number
>> being one, two, or more?

Well, one of such languages with spelling depending in number of
things is Russian, for which we have translation for git-gui, so
I guess somebody would add one for git itself.
 
> For gettex this is described at
> http://www.gnu.org/software/gettext/manual/gettext.html#Plural-forms

Which includes example for Polish language:

    1 file       =  1 plik
    2,3,4 files  =  2,3,4 pliki
    5-21 files   =  5-21  plików
    22-24 files  =  22-24 pliki
    25-31 files  =  25-31 plików
    and so on

  [...]  
  
  Three forms, special case for one and some numbers ending in 2, 3, or 4
     The header entry would look like this:

          Plural-Forms: nplurals=3; \
              plural=n==1 ? 0 : \
                     n%10>=2 && n%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2;

     Languages with this property include:

     Slavic family
          Polish

 
> Don't know how it's handled in shell scripts or perl or whatever other
> language (which does not use gettext?)

Both shell scripts and Perl scripts would use gettext: gettext.sh for
shell, Locale::Messages for Perl (we need lower level than Text::Domain,
and Locale::Maketext is first no longer recommended, and second does
not use gettext at least by default).

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: What's cooking in git.git (Feb 2011, #06; Sun, 27)

[Jeff King]

On Tue, Mar 01, 2011 at 06:07:29PM -0800, Junio C Hamano wrote:

> > On Mon, Feb 28, 2011 at 08:52:08AM -0800, Junio C Hamano wrote:
> >
> >> I am not convinced that this patch nailed that balance at exactly the
> >> right place, even though I think we are getting closer than before.  In
> >> this sequence:
> >> 
> >>     $ git checkout somewhere^0
> >>     $ git commit
> >>     $ git reset --hard somewhere_else
> >>     $ git commit
> >>     $ git checkout master
> >> ...
> > I really wouldn't expect it to help here. The problem isn't that you're
> > on a detached HEAD. It's that you're using "git reset" at all.
> 
> As you would realize later in your message, the "reset --hard" can instead
> be "checkout v1.7.3".  And the bothersome thing is that there is no safety
> against that.  We don't bother users while they are jumping around on
> detached HEAD, and it is not new; we don't say where the previous detached
> HEAD was.

No, "checkout v1.7.3" should engage the safety valve. And in your patch,
it does. So replacing your reset --hard with checkout _is_ safe. And I
think that behavior is reasonable. Reset's purpose is about shifting
HEAD to drop history, whether you are on a detached HEAD or not. Having
it warn would be annoying and pointless.

Checkout, on the other hand, is about moving your working tree to a
different place in history, and we _should_ warn about dropping history.
So it is really just a matter of educating users to use the appropriate
tool for sight-seeing (and I don't think there is much education to be
done; checkout seems like the obvious choice given it is how you got to
the first commit).

-Peff

Re: What's cooking in git.git (Feb 2011, #06; Sun, 27)

[Jeff King]

On Tue, Mar 01, 2011 at 09:24:01PM -0800, Junio C Hamano wrote:

> >   - Nit: you nicely use "%d commit%s" to handle the single/plural case
> >     in the warning message, but then you "them" later on. It needs
> >     (1 < lost) ? "them" : "it".
> 
> I actually don't like playing games like that, especially when i18n topic
> is in flight.  Among the languages I know rules reasonably well, two has
> the rule that a countable noun is spelled differently depending on the
> number of that thing is one or more, and one spells the noun the same way
> regardless of the number.  Who knows if git needs to be translated into a
> language whose noun changes its shape three-way, depending on the number
> being one, two, or more?

OK, I am showing my English-centric ignorance then, I think. :) I will
leave that topic for the i18n people to deal with.

-Peff

Re: What's cooking in git.git (Feb 2011, #06; Sun, 27)

[Ævar Arnfjörð Bjarmason]

On Wed, Mar 2, 2011 at 11:32, Jakub Narebski <jnareb@gmail.com> wrote:
> Piotr Krukowiecki <piotr.krukowiecki@gmail.com> writes:
>> Don't know how it's handled in shell scripts or perl or whatever other
>> language (which does not use gettext?)
>
> Both shell scripts and Perl scripts would use gettext: gettext.sh for
> shell, Locale::Messages for Perl (we need lower level than Text::Domain,
> and Locale::Maketext is first no longer recommended, and second does
> not use gettext at least by default).

The i18n series uses Locale::Maketext:
https://github.com/avar/git/blob/ab%2Fi18n/perl/Git/I18N.pm

What do you mean it's not recommended? There are some articles about
Perl i18n saying you shouldn't use it, effectively because:

 * Building GNU gettext is hard, let's go shopping.

 * There were some bugs in it, which do not apply to us.

So using it is fine. I might still write some Gettext::XS::Tiny module
that works with both GNU gettext and Solaris, stick it on the CPAN and
make us depend on it. It would more closely align with what we
need. But that's something for the far future.

Re: What's cooking in git.git (Feb 2011, #06; Sun, 27)

[Jakub Narebski]

On Thu, 10 Mar 2011, Ævar Arnfjörð Bjarmason wrote:
> On Wed, Mar 2, 2011 at 11:32, Jakub Narebski <jnareb@gmail.com> wrote:
> > Piotr Krukowiecki <piotr.krukowiecki@gmail.com> writes:

> > > Don't know how it's handled in shell scripts or perl or whatever other
> > > language (which does not use gettext?)
> >
> > Both shell scripts and Perl scripts would use gettext: gettext.sh for
> > shell, Locale::Messages for Perl (we need lower level than Text::Domain,
> > and Locale::Maketext is first no longer recommended, and second does
> > not use gettext at least by default).
> 
> The i18n series uses Locale::Maketext:
> https://github.com/avar/git/blob/ab%2Fi18n/perl/Git/I18N.pm

If I remember correctly previous version (maybe a few iterations in
the back) used Locale::Messages for Perl i18n, isn't it?

> What do you mean it's not recommended? There are some articles about
> Perl i18n saying you shouldn't use it, effectively because:
> 
>  * Building GNU gettext is hard, let's go shopping.

Git would use gettext for C and shell anyway, so this doesn't apply.

>  * There were some bugs in it, which do not apply to us.

   * Gettext support for plural forms etc. (ngettext) is better
     and easier to use for translators; Locale::Maketext requires one to
     be a programmer.

> So using it is fine. I might still write some Gettext::XS::Tiny module
> that works with both GNU gettext and Solaris, stick it on the CPAN and
> make us depend on it. It would more closely align with what we
> need. But that's something for the far future.

The Perl Journal article on using Locale::Maketext is supposedly outdated,
see http://rassie.org/archives/247 (On the state of i18n in Perl) from 2009.

<blockquote>
  One basic, but fatal, mistake Maketext does is off-loading a lot of
  linguistic work onto programmer.

    * One particularly important point is the plural forms support ('1 apple',
      '2 apples'), which is important for many languages outside of USA and
      Western Europe . Maketext requires you to write a quant function that
      gets a string and a number as parameters and does some voodoo to
      produce the right string. Voodoo is undefined. In gettext it is --
      a formula for producing plural forms is defined which selects one of
      provided plural phrases.

    * No translator in his sane mind will ever write a Perl module for a
      language (they aren't programmers, remember?), the programmer will
      have to do it and will also have to understand the implications.

    * The quant notation ("Your search matched [quant,_1,document]!")
      foolishly assumes word order is the same in all languages.
      Implementing a quant method properly would require passing the whole
      sentence into the function and doing a complete linguistic
      transformation which is highly non-trivial and better done by human.

    * Most of those linguistic “conventions” like number formatting or
      plural forms do not change over time and can be compiled at one place.
      One such place is Unicode’s CLDR project, which also includes plural
      form building and number/date formatting among other country- and
      language-dependant data.

    * It can’t even be assumed that the translators actually know all of
      these conventions! They might assume they know them, but translator
      is not necessarily doing translations for a living, he might be a
      volunteer, like in most open source projects. Imagine what happens
      when an amateur translator explains the inner workings of his native
      language to a programmer?

</blockquote>
-- 
Jakub Narebski
Poland