Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better ergonomic around commit form #3245

Merged
merged 4 commits into from
Apr 18, 2017

Conversation

pmiossec
Copy link
Member

@pmiossec pmiossec commented Jul 8, 2016

  • save amend checkbox state when closing form
  • disable commit form minimize button when open from browse window (because that's absolutely not ergonomic)

@KindDragon
Copy link
Contributor

save amend checkbox state when closing form

I don't know, in my use-case I often do ordinary commit after amend.

@vbjay
Copy link
Contributor

vbjay commented Jul 9, 2016

I can see remembering it till they click commit. Then reset it.

@pmiossec
Copy link
Member Author

pmiossec commented Jul 9, 2016

I don't know, in my use-case I often do ordinary commit after amend.

Not sure of the exact case you describe here...

I can see remembering it till they click commit. Then reset it.

Indeed, the state is remember until we click on "commit" but lost if we close the commit form (for example to see a commit message, a diff or something else). When we come back to the commit form to finish our commit message, we have to remember (or see!) that the checkbox was not checked anymore.

This fix just remember the state until we commit.
I think that's a better behavior...

@matkoch
Copy link
Contributor

matkoch commented Jul 13, 2016

Agree on remembering the state. Message is also restored, so let's make it consistent.

PavelKalsin pushed a commit to PavelKalsin/gitextensions that referenced this pull request Sep 21, 2016
PavelKalsin pushed a commit to PavelKalsin/gitextensions that referenced this pull request Sep 21, 2016
@ghanique ghanique mentioned this pull request Sep 21, 2016
@ghanique
Copy link

I don't want the Amend check box to be persisted if I close the form!
Sounds very dangerous!

If by mistake I create a new commit instead of amending it then that is
easily fixed. Amending a commit by mistake is much harder to fix!

@vbjay
Copy link
Contributor

vbjay commented Sep 22, 2016

The flow is this.

User checks amend
User closes form
User brings commit window back
Amend is still checked
When they commit unchecked amend

This way it still remembers the state but you never accidently amend.

@matkoch
Copy link
Contributor

matkoch commented Sep 22, 2016

@ghanique as far as I remember, you will get a message box asking if you want to amend.

@pmiossec
Copy link
Member Author

@ghanique As @vbjay and @matkoch told you, their is a popup that prevent you to do the mistake if you click too quickly.

I highly believe that there is a lot more chance that someone that just click on the amend checkbox want his choice remembered instead of wanting to disappear...

If by mistake I create a new commit instead of amending it then that is
easily fixed. Amending a commit by mistake is much harder to fix!

Not a lot. You just have to look at the reflog. And I did a pull request for that 😆 #3242

@jbialobr
Copy link
Member

jbialobr commented Apr 13, 2017

I don't know, in my use-case I often do ordinary commit after amend.

When a commit gets committed the Amend state is not remembered (it is actually cleared). So it will be only restored when you close window not committing.
@pmiossec Maybe to be super safe GitExt could remove the amend file right after reading it in?
We can also add an option to disable remembering amend state. I think it should be enabled by default.

@pmiossec
Copy link
Member Author

Maybe to be super safe GitExt could remove the amend file right after reading it in?

no problem. I will do it.

We can also add an option to disable remembering amend state. I think it should be enabled by default.

I thought about that but found out that it was a little overkill and a pitty that, what I found a (little) ergonomic improvment, be disabled by default and consequently never enabled by nearly all the users.

But I will do it, especially if that permit to merge it ;)

@jbialobr jbialobr changed the title Better ergonomic arount commit form Better ergonomic around commit form Apr 14, 2017
@jbialobr
Copy link
Member

I thought about that but found out that it was a little overkill and a pitty that, what I found a (little) ergonomic improvment, be disabled by default and consequently never enabled by nearly all the users.

As I said - it should be enabled by default.
Since there are very strong votes to not doing this, it would be good to let them disable this feature.
FYI, the amend warning can be disabled.

@pmiossec
Copy link
Member Author

As I said - it should be enabled by default.

Oh, ok! Make more sense ;-) I read twice your original sentence but understand the contrary.

I understood: add an option "disable remembering amend state" with enable state by default ;-)

I'm happy, so!

because that's absolutely not ergonomic.
In this special case, that's a lot better to close and reopen the commit form
that was the only state that remain to be saved
and that is the last impediment into closing/opening commit form
instead of minimize it
@pmiossec
Copy link
Member Author

Done! ;)

I've put the setting in Advanced > General > Don't remember 'Amend commit' checkbox on commit form close

@jbialobr jbialobr merged commit e361b5a into gitextensions:master Apr 18, 2017
@jbialobr
Copy link
Member

Thank you. I moved the setting to the Commit dialog settings page as it does not look like an advanced one.

@jbialobr jbialobr added this to the 2.50 milestone Apr 18, 2017
@pmiossec pmiossec deleted the fix_commit_form branch April 21, 2017 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants