Re: [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton

[Pratyush Yadav <me@yadavpratyush.com>]

On 12/09/19 09:35PM, Bert Wesarg wrote:
> On Wed, Sep 11, 2019 at 10:15 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> >
> > Typo in the subject. s/checketton/checkbutton/
> >
> > On 05/09/19 10:09PM, Bert Wesarg wrote:
> > > diff --git a/lib/index.tcl b/lib/index.tcl
> > > index b588db1..e07b7a3 100644
> > > --- a/lib/index.tcl
> > > +++ b/lib/index.tcl
> > > @@ -466,19 +466,19 @@ proc do_revert_selection {} {
> > >  }
> > >
> > >  proc do_select_commit_type {} {
> > > -     global commit_type selected_commit_type
> > > +     global commit_type commit_type_is_amend
> > >
> > > -     if {$selected_commit_type eq {new}
> > > +     if {$commit_type_is_amend == 0
> > >               && [string match amend* $commit_type]} {
> > >               create_new_commit
> > > -     } elseif {$selected_commit_type eq {amend}
> > > +     } elseif {$commit_type_is_amend == 1
> > >               && ![string match amend* $commit_type]} {
> >
> > Not exactly related to your change, but shouldn't these "string match
> > amend*" in the two ifs be assertions instead of checks? If
> > $commit_type_is_amend == 0, then $commit_type should _always_ be amend*,
> > and if $commit_type_is_amend == 1, then $commit_type should _never_ be
> > amend*.
> >
> 
> AFAIU this now is, that the former 'selected_commit_type' was also
> used as a request for the commit type, you set it to the desired one,
> than call do_select_commit_type, it will than check if a state change
> actually happen, and if it was to request an amend, which may also
> fail, it goes back to !amend.
> 
> Thus its not an assert.

Thanks for explaining. While I'm not the biggest fan of this design, 
let's just keep it this way for now.

-- 
Regards,
Pratyush Yadav