Checkout options for conflicts #3296
Conversation
commands/command_checkout.go
Outdated
| singleCheckout.Close() | ||
| } | ||
|
|
||
| func whichCheckout() (stage int, err error) { |
There was a problem hiding this comment.
What do you think about returning a named version of int, perhaps like:
type CheckoutStage int
const (
CheckoutStageUnknown CheckoutStage = iota
CheckoutStageBase
CheckoutStageOurs
CheckoutStageTheirs
)Potentially this could type definition could also go in package git. It looks like there is a facility from cobra that provides us a way to do this sort of "assign a value to this location in memory based not on its type" via func Var(). I think we'd have to definite an implementation of type Value interface, but that seems doable.
There was a problem hiding this comment.
Yeah, that would be nicer. I was looking for something like Var().
There was a problem hiding this comment.
Actually, I don't think Var does what we want. I remember looking at it, and now that I'm looking again, it seems to require an argument, which we aren't passing in this case. What we really need is a flag type that sets a variable to have a specific value, which I don't think cobra has.
commands/pull.go
Outdated
| // RunToPath checks out the pointer specified by p to the given path and returns | ||
| // a boolean indicating whether it was successful. It does not perform any sort | ||
| // of sanity checking or add the path to the index. | ||
| func (c *singleCheckout) RunToPath(p *lfs.WrappedPointer, path string) bool { |
There was a problem hiding this comment.
I wonder if as a preparatory patch (or series) you might want to change the return type from bool to error. It may not be wise, since I believe that the implementation below already handles the error (i.e., sends the appropriate pkt-line, or similar), but I'm not sure.
It might be useful for callers to have the information, anyway.
There was a problem hiding this comment.
This commit is new, so I can make it error instead of bool. The only reason I had to make it not return nothing is because we need to return early in Run in certain cases.
docs/man/git-lfs-checkout.1.ronn
Outdated
|
|
||
| ## DESCRIPTION | ||
|
|
||
| This command is deprecated, and should be replaced with `git checkout`. | ||
| This command is deprecated when used without `--to`, and should be replaced with |
There was a problem hiding this comment.
I wonder if it makes sense to continue to mark this command as deprecated in this case? I'm also not immediately sure what the behavior of git checkout is when given a --{ours,theirs,base} flag, and whether or not upstream runs the process filter.
There was a problem hiding this comment.
Yeah, I wasn't really sure how to go about that deprecation notice. Removing it is fine with me.
Git can store multiple different versions of a file in the index. Normally, index stage 0 is the only version provided, but if there's a conflict, there can be three additional stages. Since we'll be working with these stages in a future commit, add constants for them to the git package.
When there's a conflict with a file in Git LFS, it's difficult to get the LFS contents of the conflicted files so that they can be run through an appropriate diff tool. To make this easier, teach git lfs checkout the --base, --theirs, and --ours flags to check out the base, theirs, and ours outputs to a given path (specified with --to). Be sure not to print a deprecation message in this case, since this is not a deprecated use. Note that we use three different variables for the base, theirs, and ours flags because Cobra doesn't offer a command mode option that can parse all of the flags into one variable.
Document the --to, --base, --ours, and --theirs options to git lfs checkout. Since we're adding new functionality that isn't available by other means, stop saying the command is deprecated.
bk2204
force-pushed
the
checkout-conflict
branch
from
d52a27e to
35ba22d
Compare
When there's a conflict, it can be difficult to access the various stages of the conflict to view in a suitable diff tool. Add the
--base,--ours, and--theirsoptions to allow checking out the various stages of a conflict into different paths for easier comparison with various tools.This fixes #3258.