fix: Do not hide tabs for artificial commits #4275
Conversation
Dynamically added/hidden tabs added implementation complexity, reduced UX and caused regressions under Mono. Tabs now remain visible for artificial commits and provide minimal information. Closes gitextensions#4242
RussKie
force-pushed
the
4242_Do_not_hide_tabs
branch
from
4c5f438
to
4b5a0b1
Compare
|
Discussions in the issue on how I would have preferred the change, PR comments here. FormBrowse line 252, 253: Remove, not needed FormFileHistory: Remove line 245-255 to have the same handling as in FormBrowse. |
| var workingDir = new GitRevision(Module, GitRevision.UnstagedGuid) | ||
| { | ||
| Author = userName, |
There was a problem hiding this comment.
Fake info should be avoided. More in issue comments.
There was a problem hiding this comment.
I disagree that it is a fake info - it is the name of the individual making changes. This information comes from the gitconfig. Same applies to the email.
| AuthorEmail = userEmail, | ||
| Committer = userName, | ||
| CommitDate = DateTime.MaxValue, | ||
| CommitterEmail = userEmail, | ||
| SubjectCount = unstageCount, |
There was a problem hiding this comment.
SubjectCount is special, separate first or last in the block.
There was a problem hiding this comment.
I am afraid I don't understand what you mean.
There was a problem hiding this comment.
Information for a commit is static, except BuildInfo and SubjectCount (that is merged to Subject). Separate SubjectCount from the static fields.
There was a problem hiding this comment.
Right now GitRevision depends on an instance of GitModule which isn't static. So the artificial revision instances need to be context aware...
CheckUncommitedChanged is called once whenever the graph is refreshed, so performance hit is probably minimal.
There was a problem hiding this comment.
Yes, I suggest leave it as is. Git revision impact should be low (that the content is not static supports my point, but we already have had this discussion).
Separate CheckUncommitedChanged is removed (shared with commit button) in #4209
|
Ah yes, will optimise
Thanks
…On 27/12/2017 9:12 PM, "Gerhard Olsson" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In GitUI/UserControls/RevisionGrid.cs
<#4275 (comment)>
:
> var workingDir = new GitRevision(Module, GitRevision.UnstagedGuid)
{
+ Author = userName,
+ AuthorDate = DateTime.MaxValue,
+ AuthorEmail = userEmail,
+ Committer = userName,
+ CommitDate = DateTime.MaxValue,
+ CommitterEmail = userEmail,
SubjectCount = unstageCount,
Information for a commit is static, except BuildInfo and SubjectCount
(that is merged to Subject). Separate SubjectCount from the static fields.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4275 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEMyXjZopIcH_HRQMnPGiHAyZ8C78OWgks5tEhgCgaJpZM4RMfA8>
.
|
To be consistent with handling in gitextensions#4275
Dynamically added/hidden tabs added implementation complexity, reduced UX and caused regressions under Mono.
Tabs now remain visible for artificial commits and provide minimal information.
Closes #4242
Obsoletes #4254 #4273
Screenshots before and after (if PR changes UI):
Has been tested on (remove any that don't apply):