Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-16 Thread Ramkumar Ramachandra
Matthieu Moy wrote: Because git rebase needs multiple runs in case of conflicts. You have to store the information somewhere in the filesystem, not in a variable. What you need to store is whether you need to unstash, and where you are in the process (in Junio's version, you may be doing the

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-16 Thread Matthieu Moy
Ramkumar Ramachandra artag...@gmail.com writes: If it is 0 (which means that the rebase completely successfully), pop the stash before exiting as usual. And you're going to pop the stash even if no stash were triggered when starting the rebase. Really, think about it again: you're not going

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-16 Thread Ramkumar Ramachandra
Matthieu Moy wrote: And you're going to pop the stash even if no stash were triggered when starting the rebase. Really, think about it again: you're not going to guess whether you have to unstash without storing this information somewhere. You may argue about storing it outside the todolist,

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-16 Thread Matthieu Moy
Ramkumar Ramachandra artag...@gmail.com writes: Yes, touching a simple autostash file at stash-time, and removing it at pop-time will do. I don't see why it should be part of a (potentially user-editable) todo-list, which serves an entirely different purpose. You'll have to apply the index

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-16 Thread Matthieu Moy
Matthieu Moy matthieu@grenoble-inp.fr writes: Ramkumar Ramachandra artag...@gmail.com writes: Yes, touching a simple autostash file at stash-time, and removing it at pop-time will do. I don't see why it should be part of a (potentially user-editable) todo-list, which serves an entirely

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-16 Thread Phil Hord
On Tue, Apr 16, 2013 at 5:20 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Matthieu Moy wrote: No. Ultimately, the entry point of all these invocations is git-rebase.sh. The plan is to refactor calls from git-rebase.sh to git-rebase--*.sh scripts so that those scripts return control to

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-16 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes: Ramkumar Ramachandra artag...@gmail.com writes: If it is 0 (which means that the rebase completely successfully), pop the stash before exiting as usual. And you're going to pop the stash even if no stash were triggered when starting the

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-16 Thread Ramkumar Ramachandra
Phil Hord wrote: If I follow the latter advice about 'rm -rf', who will remember that 'rebase' had something stashed, and what will they do with it when they do? What if it is weeks or months later? I would be surprised to see long-forgotten wip show up in my workspace all of a sudden.

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Matthieu Moy
Ramkumar Ramachandra artag...@gmail.com writes: +stash_required () { + ! (git diff-files --quiet --ignore-submodules + git diff-index --quiet --cached HEAD --ignore-submodules) +} Isn't this too pessimistic? If the local changes do not overlap (in terms of files) with the pulled

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes: Ramkumar Ramachandra artag...@gmail.com writes: +stash_required () { +! (git diff-files --quiet --ignore-submodules +git diff-index --quiet --cached HEAD --ignore-submodules) +} Isn't this too pessimistic? If the local changes do

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes: Yes, that is why I said for pull-merge, --authsquash is neutral-to-better and pull.autosquash is harmful. To speak from my experience: I find myself typing git stash git pull git stash pop relatively often. Typical use-case: I start working on

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Ramkumar Ramachandra
Junio C Hamano wrote: In the end, simple cases (the canonical example being Linus keeping a local change to Name = Unicycling Gorilla in his Makefile while running dozens of git pull every day) would not be helped with this feature very much because there is rarely overlap, while in a complex

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Ramkumar Ramachandra
Matthieu Moy wrote: Isn't this too pessimistic? If the local changes do not overlap (in terms of files) with the pulled changes, then autosquash is not needed. There should be a test for the case of non-overlapping changes. In the pull-rebase case, no; it is not too pessimistic. In the

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Ramkumar Ramachandra
Matthieu Moy wrote: I disagree. A configuration option is something you set once, and then forget about. A command, or a command-line option, is something you explicitely add when you need it. You're making it out to be a much bigger difference than it actually is. Users will simply alias

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote: Okay, so you're proposing a uniform --autostash feature for both merge and rebase. Sorry, I didn't get it last time due to the typos in your email; yeah, this is worth investigating. So, I thought about this and have concluded that pull is the Right place for

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: In the end, simple cases (the canonical example being Linus keeping a local change to Name = Unicycling Gorilla in his Makefile while running dozens of git pull every day) would not be helped with this feature very much

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes: I think you're taking it the wrong way. You seem to insist that autostash should be a pull feature. I think it should be a feature of merge and rebase, and the convenience script git pull should expose them to the user. I see no reason to

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Matthieu Moy
Ramkumar Ramachandra artag...@gmail.com writes: Matthieu Moy wrote: AFAICT, git merge --abort is an alias for git reset --merge Yes, that is correct. which was precisely designed to reset only modifications comming from a merge, and not the local changes that were present before the merge

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Matthieu Moy
Ramkumar Ramachandra artag...@gmail.com writes: Matthieu Moy wrote: I disagree. A configuration option is something you set once, and then forget about. A command, or a command-line option, is something you explicitely add when you need it. You're making it out to be a much bigger

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes: Ramkumar Ramachandra artag...@gmail.com writes: Matthieu Moy wrote: AFAICT, git merge --abort is an alias for git reset --merge Yes, that is correct. which was precisely designed to reset only modifications comming from a merge, and not

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Ramkumar Ramachandra
Junio C Hamano wrote: By tempting the user to use autostash first, you are forcing the user to say reset --hard (the first one), ORIG_HEAD (to start from the pre-pull state), stash pop (to recover the autostashed WIP), to a user who got conflict during stash pop after the pull integrated the

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Ramkumar Ramachandra
Junio C Hamano wrote: git-rebase --autostash git commit --allow-empty -m 'index part' git commit --allow-empty -a -m 'working tree part' git rebase --onto $UPSTREAM git reset HEAD^ git reset --soft HEAD^

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Ramkumar Ramachandra
Matthieu Moy wrote: No, you don't. Just try if you're not convinced: Oh, I trusted the documentation on this one and never tried with a dirty worktree myself. Please fix the documentation, if you know how exactly to correct it. No, I'm not proposing to do anything for merge. There's no reason

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Ramkumar Ramachandra
Matthieu Moy wrote: No, they don't. Git forbids redefining commands with aliases. They may have an alias like git pullauto or so, but not git pull. Ofcourse, but you get the point. I use p for push, and pu for pull myself. There's not much we can do about it now, as Git cannot guess whether

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread John Keeping
On Mon, Apr 15, 2013 at 10:15:54PM +0530, Ramkumar Ramachandra wrote: Junio's criticism of pull.autostash hurting pull-merge people is cogent; my current plan is to ditch pull.autostash altogether, and implement rebase.autostash for the reduced case of a non-interactive rebase. Why restrict

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Ramkumar Ramachandra
John Keeping wrote: Why restrict it to non-interactive? I'd find it useful when doing interactive rebases as well - consider the case when you simply want to re-order some commits. Actually, I made a mistake: it should be doable for any specific rebase (includes rebase--interactive,

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: git-rebase --autostash git commit --allow-empty -m 'index part' git commit --allow-empty -a -m 'working tree part' git rebase --onto $UPSTREAM git

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Ramkumar Ramachandra
Junio C Hamano wrote: If rebase -m were to be taught to do this, the natural way to do so is to (1) Prepare the todo the usual way (2) Do those two commits for index and working tree (3) Append two insns (exec reset HEAD^ and exec reset --soft HEAD^) at the end of the rebase

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread John Keeping
On Mon, Apr 15, 2013 at 11:38:20PM +0530, Ramkumar Ramachandra wrote: Junio C Hamano wrote: If rebase -m were to be taught to do this, the natural way to do so is to (1) Prepare the todo the usual way (2) Do those two commits for index and working tree (3) Append two insns (exec

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Matthieu Moy
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: If rebase -m were to be taught to do this, the natural way to do so is to (1) Prepare the todo the usual way (2) Do those two commits for index and working tree (3) Append two insns (exec reset HEAD^ and exec reset

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Jonathan Nieder
Matthieu Moy wrote: Junio C Hamano gits...@pobox.com writes: Yes, that is why I said for pull-merge, --authsquash is neutral-to-better and pull.autosquash is harmful. To speak from my experience: I find myself typing git stash git pull git stash pop relatively often. To that end, maybe

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Jonathan Nieder
Ramkumar Ramachandra wrote: Matthieu Moy wrote: No, you don't. Just try if you're not convinced: Oh, I trusted the documentation on this one and never tried with a dirty worktree myself. Please fix the documentation, if you know how exactly to correct it. The manual says: git

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Ramkumar Ramachandra
Jonathan Nieder wrote: I wouldn't be surprised if there's still a documentation bug, though: a lack of clarity, a missing hint somewhere else, something else misleading. That seems especially likely when you say I trusted the documentation on this one. Care to point to the appropriate

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-15 Thread Jonathan Nieder
Ramkumar Ramachandra wrote: If there were uncommitted worktree changes present when the merge started, git merge --abort will in some cases be unable to reconstruct these changes. It is therefore recommended to always commit or stash your changes before running git merge. Matthieu (and

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

2013-04-14 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes: Currently, executing a 'git pull' on a dirty worktree gives the following annoying message: s/annoying//; In general, avoid forcing your value judgement to readers when you do not have to. Instead, you can just highlight the source of your