Web lists-archives.com

Re: [PATCH v1 06/11] restore: add --worktree and --index




On Fri, Mar 8, 2019 at 2:17 AM Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
>
> 'git checkout <tree-ish> <pathspec>' updates both index and
> worktree. But updating the index when you want to restore worktree
> files is non-intuitive. The index contains the data ready for the next
> commit, and there's no indication that the user will want to commit
> the restored versions.
>
> 'git restore' therefore by default only touches worktree. The user has
> the option to update either the index with
>
>     git restore --source=<tree> --index <path>  (1)
>
> or update both with
>
>     git restore --source=<tree> --index --worktree <path> (2)
>
> PS. Orignally I wanted to make worktree update default and form (1)
> would add index update while also updating the worktree, and the user
> would need to do "--index --no-worktree" to update index only. But it
> looks really confusing that "--index" option alone updates both. So
> now form (2) is used for both, which reads much more obvious.
>
> PPS. Yes form (1) overlaps with "git reset <rev> <path>". I don't know
> if we can ever turn "git reset" back to "_always_ reset HEAD and
> optionally do something else".

I'm really happy with how this series is going generally.  :-)

> +       if (!opts->checkout_worktree && !opts->checkout_index)
> +               die(_("neither '%s' or '%s' is specified"),
> +                   "--index", "--worktree");

Is this die() or BUG()?  I thought --worktree was the default.

> +               else
> +                       die(_("'%s' with only '%s' is not currently supported"),
> +                           "--patch", "--worktree");

:-(

> +               /*
> +                * NEEDSWORK: if --worktree is not specified, we
> +                * should save stat info of checked out files in the
> +                * index to avoid the next (potentially costly)
> +                * refresh. But it's a bit tricker to do...
> +                */
> +               rollback_lock_file(&lock_file);

A total tangent: I see both FIXME and NEEDSWORK in the codebase.  Are
there other 'keywords' of this type that we use?  Is there a
preference for how they are used?