Skip to content

LogView: Date column should be more fine grained #2963

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chrisrueger
Copy link

  • and show yyyy-MM-dd HH:mm:ss.SSS instead of just minute precision
  • this is useful where you want to see the exact time of log events in cases where seconds or even milliseconds matter

Shows this:

image

instead of

image

@BeckerWdf
Copy link
Contributor

This sounds like a reasonable thing to do.

Copy link
Contributor

github-actions bot commented May 9, 2025

Test Results

 1 824 files  ±0   1 824 suites  ±0   1h 31m 9s ⏱️ - 4m 34s
 7 918 tests ±0   7 690 ✅ ±0  228 💤 ±0  0 ❌ ±0 
23 841 runs  ±0  23 093 ✅ ±0  748 💤 ±0  0 ❌ ±0 

Results for commit 3f131ea. ± Comparison against base commit 0368ee5.

♻️ This comment has been updated with latest results.

@BeckerWdf
Copy link
Contributor

Does anyone from the other committers have any objections to merging this change?

@sratz
Copy link
Member

sratz commented May 9, 2025

No objections, I really like this change.

Just some technical part:
In line 109, we could now do

-return dateFormat.format(entry.getDate());
+return entry.getFormattedDate();

That does internal caching of the log date string and might optimize things a bit.

Then, we could also give org.eclipse.ui.internal.views.log.LogSession a getFormattedDate() method that returns this.

Then get rid off the additional org.eclipse.ui.internal.views.log.LogViewLabelProvider.dateFormat in the label provider.

@chrisrueger
Copy link
Author

chrisrueger commented May 9, 2025

Thanks @sratz . Like this 7e88f9f ?

Once approved, I can squash the commits at the end.

Update: Here is a screenshot grouped by Session, to see the new LogSession.getFormattedDate()

image

try {
date = formatter.parse(dateString);
} catch (ParseException e) { // do nothing
LocalDateTime ldt = LocalDateTime.parse(dateString, LOCAL_SDF);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LogEntry does

Date date = Date.from(Instant.from(GREGORIAN_SDF.parse(stringToParse)));
if (date != null) {
	fDate = date;
	fDateString = LOCAL_SDF.format(fDate.toInstant());
}

Not sure what the difference is exactly and why we need a gregorian_sdf also, but since LogEntry is the probably more mature implementation, mabye

  • make org.eclipse.ui.internal.views.log.LogEntry.GREGORIAN_SDF and org.eclipse.ui.internal.views.log.LogEntry.LOCAL_SDF package-protected
  • use the constant from over there and just use the same parsing logic here as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit more refactoring than I intended to do, but here we go :) f916646

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @chrisrueger 👍 :)

@chrisrueger chrisrueger requested a review from sratz May 9, 2025 15:00
- and show yyyy-MM-dd HH:mm:ss.SSS instead of just minute precision
- this is useful where you want to see the exact time of log events in
cases where seconds or even milliseconds matter
add and use getFormattedDate()
reuse LogEntry DateTimeFormatters
fix potential NPE

in case somebody calls getFormattedDate before setDate() was called
@chrisrueger chrisrueger force-pushed the logview-dateformat branch from a1fca3a to 3f131ea Compare May 10, 2025 11:42
@sratz sratz added this to the Next Milestone milestone May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants