Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
Conrad Irwin conrad.ir...@gmail.com writes: -i:: --include:: - Before making a commit out of staged contents so far, - stage the contents of paths given on the command line - as well. This is usually not what you want unless you - are concluding a conflicted merge. + In addition to the paths specified on the command line, + include the current contents of the index in the commit. commit is about committing what is in the index. include has always meant in addition, include the contents of listed paths in the resulting commit. The updated text looks totally the other way around. -o:: --only:: - Make a commit only from the paths specified on the - command line, disregarding any contents that have been - staged so far. This is the default mode of operation of - 'git commit' if any paths are given on the command line, - in which case this option can be omitted. - If this option is specified together with '--amend', then - no paths need to be specified, which can be used to amend - the last commit without committing changes that have - already been staged. + Only commit changes to the paths specified on the command line, + do not include the current contents of the index. This is + the default mode of operation when paths are specified. + If this option is specified with --amend it can be used + to reword the last commit without changing its contents. + This mode cannot be used with --patch or --interactive. The new text on this one does look cleaner and easier to read, at least to me, but do not include the current contents sounds as if you are recording a tree that only has Makefile and losing all the other files when you say git commit Makefile. Disregard what has been added to the index since HEAD, and only commit changes to the given paths. might be an improvement, but I dunno. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
On Sat, Oct 6, 2012 at 12:07 PM, Jeff King p...@peff.net wrote: Are you sure? Does --only mean only the changes I am about to mark or only the paths I am about to tell you about? Without partial hunk selection (i.e., commit -p), they were the same; a path you mention is a path which will be either be staged in its entirety or not. Specifying (or omitting) the path was sufficient to say what you wanted. But with -p, I can see three useful possibilities: 1. Do not include F in the commit, even if changes are staged in the index (i.e., take HEAD exactly). 2. Include F in the commit, and stage partial changes on top of what is already staged. 3. Include F in the commit, and stage partial changes on top of HEAD. In cases 2 and 3, we are still taking only the path F. But we are not taking only what is about to be staged in 2. And I can see both being useful (2 because it is more convenient not to re-approve staged changes, and 3 because there is no way to unstage changes via -p). I think I didn't consider 2. as a viable alternative because re-approving hunks is not a problem (there are typically very few hunks per file, and you'll recognise them if you've already staged them) but not being able to unstage is a big problem (as it restricts what commits I can make with --patch without changing my index). But of course we're not specifying paths. So to me it is include the changes I am about to stage via -p, as opposed to --only use the changes I am about to stage via -p. I think the current behavior is morally equivalent to how --include works with paths (which includes the paths along with the current index, rather than only committing the paths). Or am I missing something about the distinction you're making? It seems to me that the end behavior of thinking about it either way would be the same. The way I was thinking about it was to treat the index and the command line as two orthogonal parts of the commit. --include and --only control the inclusion/exclusion of the index; while the command line arguments control which (currently unstaged) things are included. This led me to the conclusion that git commit --include is equivalent to git commit, git commit --include --all is the same as git commit --all which is why I tried to change the validation logic. (You are correct that --include --only and --interactive --all still make no sense). Here's a re-roll of the patch with --only docs tweaked. Conrad 8 Clarify that --interactive/--patch add to the existing index to avoid confusion like [1]. Make explicit that --only does not work with --interactive/--patch and clean up wording around --only --amend. [1] http://thread.gmane.org/gmane.comp.version-control.git/207108 Signed-off-by: Conrad Irwin conrad.ir...@gmail.com --- Documentation/git-commit.txt | 35 +-- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 9594ac8..680d2bf 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -41,9 +41,9 @@ The content to be added can be specified in several ways: actual commit; 5. by using the --interactive or --patch switches with the 'commit' command - to decide one by one which files or hunks should be part of the commit, - before finalizing the operation. See the ``Interactive Mode'' section of - linkgit:git-add[1] to learn how to operate these modes. + to add files or hunks to the current index before committing. See the + ``Interactive Mode'' section of linkgit:git-add[1] to learn how to + operate these modes. The `--dry-run` option can be used to obtain a summary of what is included by any of the above for the next @@ -63,10 +63,14 @@ OPTIONS -p:: --patch:: - Use the interactive patch selection interface to chose - which changes to commit. See linkgit:git-add[1] for + Use the interactive patch selection interface to add hunks + to the index before committing. See linkgit:git-add[1] for details. +--interactive:: + Use the ``Interactive mode'' of linkgit:git-add[1] to edit + the index before committing. + -C commit:: --reuse-message=commit:: Take an existing commit object, and reuse the log message @@ -215,22 +219,17 @@ FROM UPSTREAM REBASE section in linkgit:git-rebase[1].) -i:: --include:: - Before making a commit out of staged contents so far, - stage the contents of paths given on the command line - as well. This is usually not what you want unless you - are concluding a conflicted merge. + In addition to the paths specified on the command line, + include the current contents of the index in the commit. -o:: --only:: - Make a commit only from the paths specified on the - command line, disregarding any contents that have been - staged so far. This is the default mode of operation of -
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
Jeff King p...@peff.net writes: Yes. The more we talk about it, the more turned off I am by the idea. Above I posed my questions as what _should_ we do when And I still think we _should_ default to --only with interactive, if we can find sane semantics. But until we can find them, it obviously does not make sense to enable it, and the whole discussion is stalled. And we must come up with an interim solution that is the least bad. Which is obviously one of: 1. Keep defaulting to --include, as that is what we have been doing. 2. Forbid the cases where it would matter (i.e., when the index and HEAD differ). The former is more convenient, but the latter is safer against future breakage. I'm OK either way, but option (1) clearly needs a documentation update. Yeah, I agree with the reasoning. This is an unessential feature that is with the problem for a long time, so let's go the route #1 first before we do anything else. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
On Sun, Oct 07, 2012 at 01:51:21PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Yes. The more we talk about it, the more turned off I am by the idea. Above I posed my questions as what _should_ we do when And I still think we _should_ default to --only with interactive, if we can find sane semantics. But until we can find them, it obviously does not make sense to enable it, and the whole discussion is stalled. And we must come up with an interim solution that is the least bad. Which is obviously one of: 1. Keep defaulting to --include, as that is what we have been doing. 2. Forbid the cases where it would matter (i.e., when the index and HEAD differ). The former is more convenient, but the latter is safer against future breakage. I'm OK either way, but option (1) clearly needs a documentation update. Yeah, I agree with the reasoning. This is an unessential feature that is with the problem for a long time, so let's go the route #1 first before we do anything else. OK. I think Conrad's patch takes us most of the way there. I had a few minor comments, but I think another round should do it. Conrad? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
Jeff King p...@peff.net writes: On Sun, Oct 07, 2012 at 01:51:21PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Which is obviously one of: 1. Keep defaulting to --include, as that is what we have been doing. 2. Forbid the cases where it would matter (i.e., when the index and HEAD differ). The former is more convenient, but the latter is safer against future breakage. I'm OK either way, but option (1) clearly needs a documentation update. Yeah, I agree with the reasoning. This is an unessential feature that is with the problem for a long time, so let's go the route #1 first before we do anything else. OK. I think Conrad's patch takes us most of the way there. I had a few minor comments, but I think another round should do it. Conrad? I'd rather want to see a patch that _only_ documents the current behaviour to unconfuse people first. I definitely do not want any patch that changes the command line parsing or any other behaviour change with problems that have to take time from reviewers to point them out mixed in it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
On Sun, Oct 07, 2012 at 03:23:31PM -0700, Junio C Hamano wrote: Yeah, I agree with the reasoning. This is an unessential feature that is with the problem for a long time, so let's go the route #1 first before we do anything else. OK. I think Conrad's patch takes us most of the way there. I had a few minor comments, but I think another round should do it. Conrad? I'd rather want to see a patch that _only_ documents the current behaviour to unconfuse people first. I definitely do not want any patch that changes the command line parsing or any other behaviour change with problems that have to take time from reviewers to point them out mixed in it. Sorry, I should have been more clear. I want to see a re-roll of only the documentation bits of Conrad's patch, for which I had only minor comments. The code part had major problems. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
Jeff King p...@peff.net writes: Actually, I am not sure that thread or feature is to blame. Certainly it would have been an opportune time to notice the problem. But this issue goes back much further for git commit --interactive, which has always assumed -i rather than -o. This even predates the switch from shell to C; you can see the same behavior from 6cbf07e (git-commit: add a --interactive option, 2007-03-05). Yes. That was after we started defaulting to only (not also) semantics when the command is run with paths, and it should also have raised a red flag to reviewers. In the case of add/commit --interactive, it is much more clear what state the index is in when the command gave interactive control to the user. The short-cut add/commit -p interface, however, does not give you an access to its s)tatus subcommand, making the user experience somewhat different. That makes the problem much more severe for -p compared to --interactive, but the fundamental UI consistency it introduces is the same as the issue under discussion in this thread. I think the right thing to do is to fix git commit -p so that it starts from the HEAD (on a temporary index), just like how partial commits are made with git commit file1 file2. Or just forbid it when the index does not match HEAD. Agreed. I am inclined to call this a bugfix, though it does worry me slightly that we would be changing a behavior that has existed for so many years. I agree it will be a bugfix, but I am afraid that the fix may have to be much more involved than start from a temporary index that matches HEAD when we are doing the '--only' semantics. Suppose you have two paths E and F, both of which have differences between HEAD and the index, and the index and the working tree file (i.e. you earlier edited E and F, did git add E F and further edited them). You say git commit -p F. What should happen? It is clear that the resulting commit should record no change since its parent commit at path E (that is what only semantics mean). What state should the add -p interaction start from for path F? Should you be picking from a patch between the state you previously git added to the index and the working tree, or should the entire difference between HEAD and the working tree eligible to be picked or deferred during the add -p session? Starting from a temporary index that matches HEAD essentially means that you lose the earlier git add F [*1*]. Another case to consider is to start from the same condition, and instead to say git commit -p without any pathspec. What should happen? Just doing use a temporary index that is initialized to HEAD may be an expedient thing to do, but I suspect that I will be saying the same I should have said 'You cannot have a pony' back then again in a not so distant future if we did so without thinking these things through. As I do not see any practical value in commit -p, I do not think it is worth my time thinking these things through thoroughly myself. Unless somebody who cares about commit -p does so to come up with reasonable semantics, and updates the code to match that desired behaviour, the responsible thing to do is to error out -p when your index is different from HEAD, I think. [Footnote] *1* A not-so-deep thinking of the above might lead to start from the index that match HEAD, except for paths specified on the pathspec given to the -p option. But I do not think it is satisfactory, either. With add -i (or commit --interactive), you have an option to selectively discard parts of your previous, overzealous git add F with its r)evert action, but because commit -p does not give an option to switch to reset -p, you can only add hunks, People who did git add E F earlier and then wants to amend that earlier add with git commit -p F, but it does not allow them to fully amend their earlier action. That is the one of the reasons why I think commit -p is a mistaken we can save one command invocation false economy that adds confusion without adding much value to the UI. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
On Fri, Oct 05, 2012 at 11:26:47PM -0700, Junio C Hamano wrote: In the case of add/commit --interactive, it is much more clear what state the index is in when the command gave interactive control to the user. The short-cut add/commit -p interface, however, does not give you an access to its s)tatus subcommand, making the user experience somewhat different. That makes the problem much more severe for -p compared to --interactive, but the fundamental UI consistency it introduces is the same as the issue under discussion in this thread. Agreed. Suppose you have two paths E and F, both of which have differences between HEAD and the index, and the index and the working tree file (i.e. you earlier edited E and F, did git add E F and further edited them). You say git commit -p F. What should happen? It is clear that the resulting commit should record no change since its parent commit at path E (that is what only semantics mean). What state should the add -p interaction start from for path F? Should you be picking from a patch between the state you previously git added to the index and the working tree, or should the entire difference between HEAD and the working tree eligible to be picked or deferred during the add -p session? Starting from a temporary index that matches HEAD essentially means that you lose the earlier git add F [*1*]. Another case to consider is to start from the same condition, and instead to say git commit -p without any pathspec. What should happen? Hmm. Good questions. In the former case, I would have said you should definitely omit E and then start from the staged point of F, as that is almost certainly what the user meant. But that is utterly inconsistent with what we are discussing for the no-pathspec case. I have a gut feeling that what I would expect for -p is roughly: 1. Feed add--interactive the current index state. 2. Feed add--interactive the set of pathspecs on the command line to limit its work. 3. For any path that is updated by the interactive session, keep the result. 4. For other paths, revert to HEAD. I think that would do what I mean most of the time. But it is a horrible set of rules to try to explain to someone (and it is off the top of my head; I wouldn't be surprised if you can come up with a situation where those rules do not behave well). Just doing use a temporary index that is initialized to HEAD may be an expedient thing to do, but I suspect that I will be saying the same I should have said 'You cannot have a pony' back then again in a not so distant future if we did so without thinking these things through. As I do not see any practical value in commit -p, I do not think it is worth my time thinking these things through thoroughly myself. Unless somebody who cares about commit -p does so to come up with reasonable semantics, and updates the code to match that desired behaviour, the responsible thing to do is to error out -p when your index is different from HEAD, I think. Yeah. I did not agree with your conclusion here when we started the conversation, but I am starting to now. I am not opposed at all to somebody working out the semantics, but I do not really care to work on it myself. In the meantime, I would rather not do any halfway fixes that will just make things worse. Another option is to leave it with -i semantics in the meantime, which are at least easy to explain: it is simply a shorthand for running git add -p git commit. That may be inconsistent with other aspects of commit, but people have (apparently) been happy with it, and there has not been a rash of complaints. As a non-user of commit -p myself, I don't have a strong opinion either way. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
Jeff King p...@peff.net writes: Another option is to leave it with -i semantics in the meantime, which are at least easy to explain: it is simply a shorthand for running git add -p git commit. That may be inconsistent with other aspects of commit, but people have (apparently) been happy with it, and there has not been a rash of complaints. Yeah, that would be the safest and possibly the sanest way forward. Did the documentation update patch by Conrad on the other subthread look sane to you? I haven't read it very carefully yet. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
On Sat, Oct 06, 2012 at 11:22:50AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Another option is to leave it with -i semantics in the meantime, which are at least easy to explain: it is simply a shorthand for running git add -p git commit. That may be inconsistent with other aspects of commit, but people have (apparently) been happy with it, and there has not been a rash of complaints. Yeah, that would be the safest and possibly the sanest way forward. Did the documentation update patch by Conrad on the other subthread look sane to you? I haven't read it very carefully yet. I didn't notice any documentation patch, and I can't find one looking through the archive. Do you have a link? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
On Sat, Oct 06, 2012 at 11:32:51AM -0700, Conrad Irwin wrote: I think I messed up sending somehow: Thanks for resending. What state should the add -p interaction start from for path F? Should you be picking from a patch between the state you previously git added to the index and the working tree, or should the entire difference between HEAD and the working tree eligible to be picked or deferred during the add -p session? Starting from a temporary index that matches HEAD essentially means that you lose the earlier git add F [*1*]. Two questions are easier answered: What should git commit --only --patch F do? = It should start you from the state of HEAD. Are you sure? Does --only mean only the changes I am about to mark or only the paths I am about to tell you about? Without partial hunk selection (i.e., commit -p), they were the same; a path you mention is a path which will be either be staged in its entirety or not. Specifying (or omitting) the path was sufficient to say what you wanted. But with -p, I can see three useful possibilities: 1. Do not include F in the commit, even if changes are staged in the index (i.e., take HEAD exactly). 2. Include F in the commit, and stage partial changes on top of what is already staged. 3. Include F in the commit, and stage partial changes on top of HEAD. In cases 2 and 3, we are still taking only the path F. But we are not taking only what is about to be staged in 2. And I can see both being useful (2 because it is more convenient not to re-approve staged changes, and 3 because there is no way to unstage changes via -p). What should git commit --include --patch F do? = It should start you from the state of the index. This one is much easier. The distinction between cases 2 and 3 above does not exist here, because we always start from the current index state. So there are two questions: 1. How does --only interact with partial staging (whether paths are specified or not)? 2. What should the default for -p be, between --only and --include? I think the answer to the second is --only; but a prerequisite to that is making --only work at all (it currently just barfs). And a prerequisite to that is figuring out what the right semantics are. The question that's harder to ponder, is what should the default be. Interestingly, I came to the exact opposite conclusion of which question is harder. :) The big UI problem with --only is not figuring out what should go in the commit, but rather ensuring that the index is in the expected state after the commit (it's the problems solved by 2888605c649ccd423232161186d72c0e6c458a48 but for hunks instead of files). If file F has hunks (H, J, K) then I stage hunk J with git add --interactive; then I commit hunks H K with git commit --interactive, the resulting index should contain H, J, K. Unfortunately, git add --interactive allows me to edit hunks, and so if I instead commit H J2 (where J2 is an edited version of J) then the index would contain (H, J) and the commit (H, J2); the working tree would contain H, J, K still. Yeah, that's a gross-ness I hadn't even considered. All in all, I think supporting --only --interactive is well beyond what I'm capable of doing, and probably pushing the limits of what's sane. (it would be nice for warm fuzzy completeness reasons though). Yes. The more we talk about it, the more turned off I am by the idea. Above I posed my questions as what _should_ we do when And I still think we _should_ default to --only with interactive, if we can find sane semantics. But until we can find them, it obviously does not make sense to enable it, and the whole discussion is stalled. And we must come up with an interim solution that is the least bad. Which is obviously one of: 1. Keep defaulting to --include, as that is what we have been doing. 2. Forbid the cases where it would matter (i.e., when the index and HEAD differ). The former is more convenient, but the latter is safer against future breakage. I'm OK either way, but option (1) clearly needs a documentation update. On Fri, Oct 5, 2012 at 3:57 PM, Jeff King p...@peff.net wrote: We should probably also support explicit -i -p and -o -p options, as well (the former would give people who really want the existing behavior a way to get it). And the same for --interactive. I can't say I'm excited about making all that work, though. Like you, I think it is more sane to use existing tools to inspect and tweak the index to your liking, and then commit. You made the same thinko as me :). --include isn't defined to mean include the index as well, but rather include these files when committing the index. Flipping that around makes a lot of sense and then --include can be used semantically with --patch, --interactive or even --all. (patch attached). But of course we're not specifying paths. So to me it is include the changes I am about to
git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
What I did: - New file images/coins.asy ~~- 'git add images/coins.asy' - Started adding new stuff to fg.tex - Noticed a old bug in fg.tex, fixed that one - Did 'git -pm Some message' and selected just the bugfix But git created a commit _including_ the new file. Tried to go back: - 'git reset HEAD^' Now the new file isn't staged anymore What I expected to happen: - Only the explicitly selected chunks commited - No losing staged changes -- Dr. Horst H. von Brand User #22616 counter.li.org Departamento de InformaticaFono: +56 32 2654431 Universidad Tecnica Federico Santa Maria +56 32 2654239 Casilla 110-V, Valparaiso, Chile 234 Fax: +56 32 2797513 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
On Fri, 05 Oct 2012 16:20:45 +0200, Horst H. von Brand vonbr...@inf.utfsm.cl wrote: What I did: - New file images/coins.asy ~~- 'git add images/coins.asy' - Started adding new stuff to fg.tex - Noticed a old bug in fg.tex, fixed that one - Did 'git -pm Some message' and selected just the bugfix But git created a commit _including_ the new file. Tried to go back: Exactly what's supposed to happen. git add tells git you want to add the file to the index. The index is what you're going to commit later on. So what you did there was - Tell git to add images/coins.asy to the next commit - hack hack hack - fix old_bug - Add old_bug chunks of code to next commit create commit - 'git reset HEAD^' Now the new file isn't staged anymore What I expected to happen: - Only the explicitly selected chunks commited - No losing staged changes As explained above, you didn't lose staged changes, you staged more changes and committed. Then you use git reset to go back to the state of HEAD^, where the file wasn't tracked and therefore not staged either. So you're back at square one[1], commit the bug fix, then add the bugfixes in a commit and stage the new file for inclusion in your next commit. Hope this helps, Frans [1] Arguably two, since you still have changes lying around. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
Frans Klaver franskla...@gmail.com writes: On Fri, 05 Oct 2012 16:20:45 +0200, Horst H. von Brand vonbr...@inf.utfsm.cl wrote: What I did: - New file images/coins.asy ~~- 'git add images/coins.asy' - Started adding new stuff to fg.tex - Noticed a old bug in fg.tex, fixed that one - Did 'git -pm Some message' and selected just the bugfix But git created a commit _including_ the new file. Tried to go back: Exactly what's supposed to happen. git add tells git you want to add the file to the index. The index is what you're going to commit later on. Assuming that the last step of what Horst did was git commit -pm, I think Git is wrong in this case. When you tell git commit what to commit, unless you give -i (aka also) option, the command makes a commit to record changes only from what you tell git commit to commit, regardless of what you earlier did to the index. And choosing what to add via the interactive interface is in the same spirit as telling what to commit to git commit, so it should behave the same. This is one of the times I wish I said No, you cannot have a pony. The change was done without thinking things through, and reviewers including me did not realize this particular downside. My accepting this misfeature (or a poorly implemented feature that has a potential to be useful) was essentially me saying: When making a commit that does not match my working tree state, I always check with diff --cached to make sure what I think I am committing matches what I am committing, so I won't use such a lazy option myself. I am not excited to think things through to see what possible pitfalls the feature may have for you; I'll let you guys hang yourself with that long rope. And we are seeing a backfire from that not bothering to think things thorough. I think the right thing to do is to fix git commit -p so that it starts from the HEAD (on a temporary index), just like how partial commits are made with git commit file1 file2. Or just forbid it when the index does not match HEAD. Cf. http://thread.gmane.org/gmane.comp.version-control.git/173033/focus=173246 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git 1.8.0.rc0.18.gf84667d trouble with git commit -p file
On Fri, Oct 05, 2012 at 03:29:10PM -0700, Junio C Hamano wrote: Assuming that the last step of what Horst did was git commit -pm, I think Git is wrong in this case. When you tell git commit what to commit, unless you give -i (aka also) option, the command makes a commit to record changes only from what you tell git commit to commit, regardless of what you earlier did to the index. Yeah. Defaulting to -o would match the rest of git-commit's behavior much better. This is one of the times I wish I said No, you cannot have a pony. The change was done without thinking things through, and reviewers including me did not realize this particular downside. [...] Cf. http://thread.gmane.org/gmane.comp.version-control.git/173033/focus=173246 Actually, I am not sure that thread or feature is to blame. Certainly it would have been an opportune time to notice the problem. But this issue goes back much further for git commit --interactive, which has always assumed -i rather than -o. This even predates the switch from shell to C; you can see the same behavior from 6cbf07e (git-commit: add a --interactive option, 2007-03-05). I guess you could argue that --interactive and --patch should have different defaults, but I'm not sure I agree. They should both match what git commit foo does by default. I think the right thing to do is to fix git commit -p so that it starts from the HEAD (on a temporary index), just like how partial commits are made with git commit file1 file2. Or just forbid it when the index does not match HEAD. Agreed. I am inclined to call this a bugfix, though it does worry me slightly that we would be changing a behavior that has existed for so many years. We should probably also support explicit -i -p and -o -p options, as well (the former would give people who really want the existing behavior a way to get it). And the same for --interactive. I can't say I'm excited about making all that work, though. Like you, I think it is more sane to use existing tools to inspect and tweak the index to your liking, and then commit. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
