tibirna / qgit

Official git repository for QGit.
Other
174 stars 68 forks source link

FileHistory: Add "Short Hash" column. #119

Closed WickedSmoke closed 2 years ago

WickedSmoke commented 2 years ago

This is a basic implementation for issue #118. The hash size is hardcoded to show 8 characters.

tibirna commented 2 years ago

Thank you very much for your very quick addition. Much appreciated. I made a small comment on the patch. Please let me know if you don't agree.

WickedSmoke commented 2 years ago

The patch has been updated to get the ideal length from Git. I stored shortHashLen in the Git class so that FileHistory did not become coupled to MainImpl. This does fix the execution error dialog mentioned above.

tibirna commented 2 years ago

Merged. Thank you very much for your contribution!

As is my custom, I added your name to the list of contributors. Thanks again!

Future improvement of this new feature would be, IMO:

WickedSmoke commented 2 years ago

Heh, thanks for pulling, but you jumped the gun a bit. I was wondering if the column should be named "Commit", as this is the word used by the CLI program and a number of other GUIs.

tibirna commented 2 years ago

If you want to continue on this, please do. I'll wait your signal before re-merging.

WickedSmoke commented 2 years ago

I grouped the hash column with Author & Date to match the grouping in the CLI log and also place it near the hash QLineEdit so that the eye has to travel less when verifying that the list item and the line edit values match. Other GUIs do put Commit before the Short Log.

I'm not going to make the column optional as part of this patch. This can be done if someone else wants to make the lists columns fully configurable (both order & visibility).

WickedSmoke commented 2 years ago

If you agree that the column name should be "Commit" I'll do that and call it a day.

tibirna commented 2 years ago

I agree. Thanks again.

On Sun, 7 Nov 2021 at 12:04, WickedSmoke @.***> wrote:

If you agree that the column name should be "Commit" I'll do that and call it a day.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/tibirna/qgit/pull/119#issuecomment-962646154, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJOWDTWH4JZGTBRWPRT35TUK2WSLANCNFSM5HQEYE2Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

WickedSmoke commented 2 years ago

I overwrote my master commit with the column rename so please pull that.

tibirna commented 2 years ago

Done. Thanks.

pwillard2 commented 2 years ago

I got zip file, and with this Commit change, I am seeing 2 columns with "Author Date". Apparently "Author" column has been renamed. I think you need to change the index in FileHistory::clear to use 5 instead of 4:

if (testFlag(REL_DATE_F)) { secs = QDateTime::currentDateTime().toTime_t(); headerInfo[5] = "Last Change"; } else { secs = 0; headerInfo[5] = "Author Date"; } rowCnt = revOrder.count(); annIdValid = false; endResetModel(); emit headerDataChanged(Qt::Horizontal, 0, 5);

tibirna commented 2 years ago

You are right, M. Willard. Thanks very much! I completely missed this.

It is fixed now at the top of master.

WickedSmoke commented 2 years ago

Those headerInfo indices should be using the ColumnType enums so they stay synced with future changes.

tibirna commented 2 years ago

@WickedSmoke you are absolutely right. Done. Thanks.