Bug 469385 - Show modified date in git staging view 'changes tables'
Summary: Show modified date in git staging view 'changes tables'
Status: NEW
Alias: None
Product: EGit
Classification: Technology
Component: UI (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-04 07:55 EDT by Steven Spungin CLA
Modified: 2015-06-10 02:00 EDT (History)
2 users (show)

See Also:


Attachments
screenshot (84.22 KB, image/png)
2015-06-04 07:55 EDT, Steven Spungin CLA
no flags Details
actual screenshot (64.18 KB, image/png)
2015-06-08 15:40 EDT, Steven Spungin CLA
no flags Details
actual screenshot (tree table) (78.70 KB, image/png)
2015-06-08 15:42 EDT, Steven Spungin CLA
no flags Details
actual screenshot version 2 (106.62 KB, image/png)
2015-06-08 22:27 EDT, Steven Spungin CLA
no flags Details
menu with relative date option (52.60 KB, image/png)
2015-06-08 22:28 EDT, Steven Spungin CLA
no flags Details
Example repo for performance checks (84.49 KB, application/zip)
2015-06-09 18:06 EDT, Andrey Loskutov CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Steven Spungin CLA 2015-06-04 07:55:31 EDT
It would be very useful to see the local modified date as a column in the 'git staging view'.
Comment 1 Steven Spungin CLA 2015-06-04 07:55:55 EDT
Created attachment 254106 [details]
screenshot
Comment 2 Matthias Sohn CLA 2015-06-08 05:52:37 EDT
good idea

how about contributing this improvement ;-)
Comment 3 Steven Spungin CLA 2015-06-08 10:06:10 EDT
For the Eclipse Foundation, we no longer upload gerrit reviews until the idea is approved in the bug report by a commiter for the specified project.  

@Mattias, Are you willing and able to be active in reviewing this patch?
Comment 4 Matthias Sohn CLA 2015-06-08 11:58:05 EDT
(In reply to Steven Spungin from comment #3)
> For the Eclipse Foundation, we no longer upload gerrit reviews until the
> idea is approved in the bug report by a commiter for the specified project.  

I am an EGit committer

> @Mattias, Are you willing and able to be active in reviewing this patch?

+1
Comment 5 Steven Spungin CLA 2015-06-08 15:40:41 EDT
Created attachment 254199 [details]
actual screenshot
Comment 6 Steven Spungin CLA 2015-06-08 15:42:13 EDT
Created attachment 254200 [details]
actual screenshot (tree table)
Comment 7 Matthias Sohn CLA 2015-06-08 16:44:29 EDT
Looks good, the date format should match the one shown in history view, also there should be an option to show relative dates.

I think there is no point in supporting different date format settings in different EGit views hence I think we should generalize the existing preference which is currently used by the history view
https://git.eclipse.org/r/#/c/3362/
and "select a commit" dialog
https://git.eclipse.org/r/#/c/23175/
and also use it in the staging view.

The basic date formatter is available in JGit, see the above changes how this is used.

Like in the history view the staging view's quick menu (click on the little triangle in the top right corner of the view) should allow to toggle if local date/time or relative date is shown
Comment 8 Steven Spungin CLA 2015-06-08 21:13:10 EDT
I can't seem to find a simple way to get the modification date from the index/stage StagingEntry.  I was expecting some kind of an interface from that object to access the metadata.  Any ideas?
Comment 9 Steven Spungin CLA 2015-06-08 21:59:22 EDT
>I can't seem to find a simple way to get the modification date from the index/stage StagingEntry.

OK.  I got that working.  I had to implement IndexFileRevision.getTimestamp.  It was returning -1. Is that by design?  If so, I can put the fix in another method instead.


Thanks for the date format links.
Comment 10 Steven Spungin CLA 2015-06-08 22:27:54 EDT
Created attachment 254215 [details]
actual screenshot version 2
Comment 11 Steven Spungin CLA 2015-06-08 22:28:17 EDT
Created attachment 254216 [details]
menu with relative date option
Comment 12 Eclipse Genie CLA 2015-06-08 23:32:21 EDT
New Gerrit change created: https://git.eclipse.org/r/49727
Comment 13 Andrey Loskutov CLA 2015-06-09 18:06:42 EDT
Created attachment 254258 [details]
Example repo for performance checks

First of all, independently of provided patch, staging view performs not so good if the number of files exceeds ~5000. Please play with (extreme) attached project example. This is not every day use case, but I've seen commits where the old code were removed or new added where number of files was even over 20000.

Unfortunately, provided patch (https://git.eclipse.org/r/#/c/49727/4) adds the performance overhead of about 75-100%. On my Intel I5 with 4 cores clocked at 2.6GHz with 10000 files added to the index the UI freezes for 7-8 seconds without patch and about 14-15 seconds with the patch (switching between projects in different repositories).

This is going worse if all file types are different (because of different icons, times for just listing the files are not acceptable with and without the patch, around 30 - 45 seconds).

The scenario I have in mind is a merge of a huge commits or removal of obsoleted code. Unfortunately sometimes we have such commits in our repo, and this will definitely kill UI.

So if there will be not better performance, it would be better to make this feature optional - or try to fix table performance of the staging view first (use "virtual" flag for the table may be?).

Other (smaller) issues:
 * Deleted files always have "45 years ago" or "1970-01-01..." date. I don't think it makes sense to show this to user.
 * "File" column should probably better be renamed to "Name"?
 * having column headers make sense only if they can be used to change sort order (by name/date). So I would just hide them - they only need extra space and don't look nice.
 * Please for the new code always add {} braces, see https://wiki.eclipse.org/EGit/Contributor_Guide#Braces_for_one-line_statements
Comment 14 Steven Spungin CLA 2015-06-10 02:00:36 EDT
The posted patch takes less then 1 second to load the 'Huge Repo' on my machine. (10000 files) I extended the test to 50000 files.  It takes around 1 second to switch the repo into the staging view.  The staging view is using the index file for timestamps, not the filesystem.  

I do call dirCache.read() for every entry, but the documentation states it is optimized for multiple calls.  Perhaps that's the issue on your machine.  I will change the code to only load it on refresh; maybe that will improve things.

What OS are you using?

---

> * having column headers make sense only if they can be used to change sort order (by name/date). So I would just hide them - they only need extra space and don't look nice.

You need headers to resize the columns, or to change the column order (although that is not implemented yet).

---

I do think that adding a preference to 'show timestamps' would be a good thing.