| Message ID | 20190329103919.15642-16-pclouds@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add new command 'switch' | expand |
On 29/03/2019 10:39, Nguyễn Thái Ngọc Duy wrote: > --discard-changes is a better name than --force for this option since > it's what really happens. I didn't realize when I suggested the name that --force overwrites untracked files as well as discarding changes from tracked files. I think we should document that. It would be nice if read-tree --reset -u took an optional argument so read-tree --reset=tracked -u would not overwrite untracked files. Then we could have --discard-changes just discard the changes and not overwrite untracked files. I had a quick look at unpack trees and it looks like a fairly straight forward change (famous last words) - perhaps I'll have a go at it next week. Best Wishes Phillip > --force is turned to an alias for > --discard-changes. But it's meant to be an alias for potentially more > force options in the future. > > Side note. It's not obvious from the patch but --discard-changes also > affects submodules if --recurse-submodules is used. The knob to force > updating submodules is hidden behind unpack-trees.c > > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > builtin/checkout.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/builtin/checkout.c b/builtin/checkout.c > index 319ba372e3..6d0b2ef565 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -53,6 +53,7 @@ struct checkout_opts { > int count_checkout_paths; > int overlay_mode; > int no_dwim_new_local_branch; > + int discard_changes; > > /* > * If new checkout options are added, skip_merge_working_tree > @@ -680,7 +681,7 @@ static int merge_working_tree(const struct checkout_opts *opts, > return error(_("index file corrupt")); > > resolve_undo_clear(); > - if (opts->force) { > + if (opts->discard_changes) { > ret = reset_tree(get_commit_tree(new_branch_info->commit), > opts, 1, writeout_error); > if (ret) > @@ -802,7 +803,7 @@ static int merge_working_tree(const struct checkout_opts *opts, > if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) > die(_("unable to write new index file")); > > - if (!opts->force && !opts->quiet) > + if (!opts->discard_changes && !opts->quiet) > show_local_changes(&new_branch_info->commit->object, &opts->diff_options); > > return 0; > @@ -1309,6 +1310,9 @@ static int checkout_branch(struct checkout_opts *opts, > if (opts->force && opts->merge) > die(_("'%s' cannot be used with '%s'"), "-f", "-m"); > > + if (opts->discard_changes && opts->merge) > + die(_("'%s' cannot be used with '%s'"), "--discard-changes", "--merge"); > + > if (opts->force_detach && opts->new_branch) > die(_("'%s' cannot be used with '%s'"), > "--detach", "-b/-B/--orphan"); > @@ -1445,6 +1449,8 @@ static int checkout_main(int argc, const char **argv, const char *prefix, > opts->merge = 1; /* implied */ > git_xmerge_config("merge.conflictstyle", opts->conflict_style, NULL); > } > + if (opts->force) > + opts->discard_changes = 1; > > if ((!!opts->new_branch + !!opts->new_branch_force + !!opts->new_orphan_branch) > 1) > die(_("-b, -B and --orphan are mutually exclusive")); > @@ -1600,6 +1606,8 @@ int cmd_switch(int argc, const char **argv, const char *prefix) > N_("create and switch to a new branch")), > OPT_STRING('C', "force-create", &opts.new_branch_force, N_("branch"), > N_("create/reset and switch to a branch")), > + OPT_BOOL(0, "discard-changes", &opts.discard_changes, > + N_("throw away local modifications")), > OPT_END() > }; > int ret; >
On Thu, Apr 25, 2019 at 5:02 PM Phillip Wood <phillip.wood123@gmail.com> wrote: > > On 29/03/2019 10:39, Nguyễn Thái Ngọc Duy wrote: > > --discard-changes is a better name than --force for this option since > > it's what really happens. > > I didn't realize when I suggested the name that --force overwrites > untracked files as well as discarding changes from tracked files. I > think we should document that. It would be nice if read-tree --reset -u > took an optional argument so read-tree --reset=tracked -u would not > overwrite untracked files. Then we could have --discard-changes just > discard the changes and not overwrite untracked files. I had a quick > look at unpack trees and it looks like a fairly straight forward change > (famous last words) - perhaps I'll have a go at it next week. So, --discard-changes is all about tracked changes, and we may have --overwrite-untracked to cover the other part, and --force enables both? That does not sound so bad (and maybe a good cure for those "overwriting untracked" reports we've seen quite often lately). Good luck with unpack-trees.c. But if it turns out you're too busy, just let me know if want to hand that back to me.
On 25/04/2019 11:12, Duy Nguyen wrote: > On Thu, Apr 25, 2019 at 5:02 PM Phillip Wood <phillip.wood123@gmail.com> wrote: >> >> On 29/03/2019 10:39, Nguyễn Thái Ngọc Duy wrote: >>> --discard-changes is a better name than --force for this option since >>> it's what really happens. >> >> I didn't realize when I suggested the name that --force overwrites >> untracked files as well as discarding changes from tracked files. I >> think we should document that. It would be nice if read-tree --reset -u >> took an optional argument so read-tree --reset=tracked -u would not >> overwrite untracked files. Then we could have --discard-changes just >> discard the changes and not overwrite untracked files. I had a quick >> look at unpack trees and it looks like a fairly straight forward change >> (famous last words) - perhaps I'll have a go at it next week. > > So, --discard-changes is all about tracked changes, and we may have > --overwrite-untracked to cover the other part, and --force enables > both? I was thinking of --discard-changes dealing with tracked changes and having --force for untracked changes as well. I'm not sure we need --overwrite-untracked for switch, just for read-tree. That does not sound so bad (and maybe a good cure for those > "overwriting untracked" reports we've seen quite often lately). > > Good luck with unpack-trees.c. But if it turns out you're too busy, > just let me know if want to hand that back to me. Thanks, I've got something working, I'll clean it up and send it later in the week. Best Wishes Phillip
diff --git a/builtin/checkout.c b/builtin/checkout.c index 319ba372e3..6d0b2ef565 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -53,6 +53,7 @@ struct checkout_opts { int count_checkout_paths; int overlay_mode; int no_dwim_new_local_branch; + int discard_changes; /* * If new checkout options are added, skip_merge_working_tree @@ -680,7 +681,7 @@ static int merge_working_tree(const struct checkout_opts *opts, return error(_("index file corrupt")); resolve_undo_clear(); - if (opts->force) { + if (opts->discard_changes) { ret = reset_tree(get_commit_tree(new_branch_info->commit), opts, 1, writeout_error); if (ret) @@ -802,7 +803,7 @@ static int merge_working_tree(const struct checkout_opts *opts, if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) die(_("unable to write new index file")); - if (!opts->force && !opts->quiet) + if (!opts->discard_changes && !opts->quiet) show_local_changes(&new_branch_info->commit->object, &opts->diff_options); return 0; @@ -1309,6 +1310,9 @@ static int checkout_branch(struct checkout_opts *opts, if (opts->force && opts->merge) die(_("'%s' cannot be used with '%s'"), "-f", "-m"); + if (opts->discard_changes && opts->merge) + die(_("'%s' cannot be used with '%s'"), "--discard-changes", "--merge"); + if (opts->force_detach && opts->new_branch) die(_("'%s' cannot be used with '%s'"), "--detach", "-b/-B/--orphan"); @@ -1445,6 +1449,8 @@ static int checkout_main(int argc, const char **argv, const char *prefix, opts->merge = 1; /* implied */ git_xmerge_config("merge.conflictstyle", opts->conflict_style, NULL); } + if (opts->force) + opts->discard_changes = 1; if ((!!opts->new_branch + !!opts->new_branch_force + !!opts->new_orphan_branch) > 1) die(_("-b, -B and --orphan are mutually exclusive")); @@ -1600,6 +1606,8 @@ int cmd_switch(int argc, const char **argv, const char *prefix) N_("create and switch to a new branch")), OPT_STRING('C', "force-create", &opts.new_branch_force, N_("branch"), N_("create/reset and switch to a branch")), + OPT_BOOL(0, "discard-changes", &opts.discard_changes, + N_("throw away local modifications")), OPT_END() }; int ret;
--discard-changes is a better name than --force for this option since it's what really happens. --force is turned to an alias for --discard-changes. But it's meant to be an alias for potentially more force options in the future. Side note. It's not obvious from the patch but --discard-changes also affects submodules if --recurse-submodules is used. The knob to force updating submodules is hidden behind unpack-trees.c Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/checkout.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)