Re: [PATCH] docs: give more hints about how "add -e" works

On Thu, Oct 21, 2010 at 03:32:15PM -0700, Junio C Hamano wrote:

quoted

+The intent of this option is to pick and choose lines of the patch to
+apply, or even to modify the contents of lines to be staged. There are
+three line types in a patch: addition lines (beginning with a plus),
+removal lines (beginning with a minus), and context lines (beginning
+with a space). In general, it should be safe to:
++
+--
+* remove addition lines (don't stage the line)

I am not sure if the use of the word "stage" here is correct, even when
read from the "git stage" lovers' viewpoint.

If the "+" line is a pure addition without any corresponding line in the
preimage (which is removed by "-"), then this is "Don't add that line".
If it has a corresponding "-" line somewhere, that is rather "Remove
the corresponding line in the preimage".

"Don't add the updated line" might be a good compromise.

I'm not sure that using the word "add" here is really any different than
"stage". Because we are really talking about adding _to_ the index,
which to me is the same as staging. IOW, I could say "Don't stage the
updated line" and it would mean the exact same thing.

It is a separate issue that adding/staging that line may not be a true
addition, but rather "don't add the updated line". That was what I was
trying to address in the final paragraph.

Perhaps the list should be structured not as "what you can do to each
line" but rather "here are some _concepts_ you might see, here's how
they are represented, and how you might want to edit them". So
basically:

  - added lines; represented by "+" lines. You can prevent staging any
    addition lines by deleting them.

  - removed lines; represented by "-" lines. You can prevent staging any
    removal lines by converting "-" to " ".

  - modified lines; represented by "-" followed by "+". You can prevent
    staging the modification by converting the "-" to a " ", and
    removing the "+" lines. And this would be a good place to warn that
    just deleting half of the pair is going to cause problems.

  - existing lines; represented by " " lines. You can:

      - remove them, by converting " " to "-".

      - modify them, by converting " " to "-", and adding a new "+" line
        with the new content.

  - adding new lines; do not yet exist in the patch. You can add new
    lines by inserting "+" lines with the new content.

which is perhaps better, as it directs the user according to what they
actually want to accomplish.

I shied away from that intially because there are some more complex
cases (e.g., added lines next to modified lines in the same hunk). But I
expect anybody with half a clue can probably figure out how to handle
that using the primitives outlined above.

quoted

+Similarly, your patch will likely not apply if you:
++
+--
+* add context or removal lines
+* delete removal or context lines
+* modify the contents of context or removal lines
+--
++
+NOTE: In the first list above, the results given for each action are
+with respect to that patch line only. Conceptual changes like
+modification of a line in the original file are actually represented by
+removal of the old line followed by addition of the new line. Deleting
+only the addition line of this pair but leaving the removal line would
+therefore convert the modification into a deletion. In other words, use
+this feature with caution, as it is easy to stage unintended changes.

Is there a way to move this note way upwards?  Once the reader understands
what this paragraph teaches, it becomes much easier to understand the
implication of "remove addition".

I agree it would be better at the top, but I think formatting it as I
just wrote above would mean we can actually explain the issue in a more
appropriate place. And then this bottom warning can just go away. I
think I may still put a few words of caution at the top.

-Peff