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

[git] Git history shows incorrect commit date #1754

Closed
kittaakos opened this issue Apr 20, 2018 · 11 comments
Closed

[git] Git history shows incorrect commit date #1754

kittaakos opened this issue Apr 20, 2018 · 11 comments
Labels
beginners issues that are perfect for beginners bug bugs found in the application git issues related to git

Comments

@kittaakos
Copy link
Contributor

screen shot 2018-04-20 at 10 01 33

One has to call:

date.setUTCSeconds(Number.parseInt(time));

on the date when converting the epoch time to date.

Here: https://github.com/theia-ide/theia/blob/25c91c41a93b9e4f51786f7d9b5fca6a7bd19525/packages/git/src/browser/history/git-history-widget.ts#L113

@kittaakos kittaakos added bug bugs found in the application beginners issues that are perfect for beginners git issues related to git labels Apr 20, 2018
@AlexErling
Copy link

Hello, I am new to open source and was wondering if I could claim this issue.
Thanks,
Alex

@kittaakos
Copy link
Contributor Author

Absolutely. It's yours. Let me know if you need help.

@AlexErling
Copy link

Hi, I could use a little help. Any information to point me in the right direction would be appreciated.
What type is the commit.author.timestamp, a string or a number? I am confused where to call
date.setUTCSeconds(Number.parseInt(time));
I was thinking something along the lines of
date.setUTCSeconds(Number.parseInt(commit.author.timestamp)

Any guidance would be great.

@kittaakos
Copy link
Contributor Author

commit.author.timestamp

It is a number. See:
https://github.com/theia-ide/theia/blob/a62d8eaa8434947777de09d3e2b28be948744b2b/packages/git/src/common/git-model.ts#L316

Here is a small snippet depicting the problem:

const t = 1512511924;
console.log(new Date(t)); // Sun Jan 18 1970 13:08:31 GMT+0100 (CET)
const d = new Date(t);
d.setUTCSeconds(t);
console.log(d); // Sat Dec 23 2017 11:20:04 GMT+0100 (CET)

You need to do something like this:

diff --git a/packages/git/src/browser/history/git-history-widget.ts b/packages/git/src/browser/history/git-history-widget.ts
index 64e74f9d..b6c76107 100644
--- a/packages/git/src/browser/history/git-history-widget.ts
+++ b/packages/git/src/browser/history/git-history-widget.ts
@@ -108,9 +108,11 @@ export class GitHistoryWidget extends GitNavigableListWidget<GitHistoryListNode>
                     for (const commit of changes) {
                         const fileChangeNodes: GitFileChangeNode[] = [];
                         const avatarUrl = await this.avartarService.getAvatar(commit.author.email);
+                        const authorDate = new Date(commit.author.timestamp);
+                        authorDate.setUTCSeconds(commit.author.timestamp);
                         commits.push({
                             authorName: commit.author.name,
-                            authorDate: new Date(commit.author.timestamp),
+                            authorDate,
                             authorEmail: commit.author.email,
                             authorDateRelative: commit.authorDateRelative,
                             authorAvatar: avatarUrl,

@kittaakos
Copy link
Contributor Author

And if you are there, perhaps, you could fix the typo in avartarService. It should be avatarService instead.

@AlexErling
Copy link

Okay, here are the changes I made then:

const avatarUrl = await this.avatarService.getAvatar(commit.author.email);
const authorDate = new Date(commit.author.timestamp);
authorDate.setUTCSeconds(commit.author.timestamp);
commits.push({
    authorName: commit.author.name,
    authorDate: authorDate,

@kittaakos
Copy link
Contributor Author

Looks good. FYI, you can shorten the object literal with this:

const avatarUrl = await this.avatarService.getAvatar(commit.author.email);
const authorDate = new Date(commit.author.timestamp);
authorDate.setUTCSeconds(commit.author.timestamp);
commits.push({
    authorName: commit.author.name,
    authorDate,

If you would like to contribute, you have to provide your changes with a pull request and sign it. You can find additional details about it here. If you need help, forking, creating a PR or anything; do not hesitate to ask. I am happy to help you with the onboarding.

@AlexErling
Copy link

Okay, I sent in the PR. Let me know if there is something else I need to do. Also, let me know if you know of anything else or any other projects I could help out with. I would love to help out and learn more if possible!

@svenefftinge
Copy link
Contributor

svenefftinge commented Apr 30, 2018

@kittaakos Could you elaborate why and how setUTCSeconds fix this? Also in https://github.com/theia-ide/theia/blob/a62d8eaa8434947777de09d3e2b28be948744b2b/packages/git/src/common/git-model.ts#L316 it is unclear what to expect. Is it the number of milliseconds since 01-01-1970 ? What timezone? Would be better to have an ISO string there.

@kittaakos
Copy link
Contributor Author

We use --date=unix to get the back the author-date of the commit.

--date=unix shows the date as a Unix epoch timestamp (seconds since 1970). As with --raw, this is always in UTC and therefore -local has no effect.

AFAIK, one can convert the epoch time to JS date either with setUTCSeconds(epoch) or by using the constructor of date new Date(setUTCSeconds * 1000). I am OK with both approaches or any better one too.

Would be better to have an ISO string there.

Let's do that; anything would be better than 1970.

@kittaakos
Copy link
Contributor Author

Do you have any updates on this, @svenefftinge or can we update the Git API (changing the epoch number to the desired string) in a separate ticket? If the fix is right, I do the review. Thanks!

vince-fugnitto added a commit that referenced this issue Nov 6, 2018
Fixes #1770
Fixes #1754

- Fixes error thrown when attempting to perform `Date.toLocaleDateString` in the `git-commit-detail-widget`.
- Fixes error thrown when attempting to iterate over `fileChangeNodes` and it is `undefined`.

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
vince-fugnitto added a commit that referenced this issue Nov 7, 2018
Fixes #1770
Fixes #1754

- Fixes error thrown when attempting to perform `Date.toLocaleDateString` in the `git-commit-detail-widget`.
- Fixes error thrown when attempting to iterate over `fileChangeNodes` and it is `undefined`.
- Fix typo when no git history is available
- Simplify logic in `addCommits`

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
vince-fugnitto added a commit that referenced this issue Nov 7, 2018
Fixes #1770
Fixes #1754

- Fixes error thrown when attempting to perform `Date.toLocaleDateString` in the `git-commit-detail-widget`.
- Fixes error thrown when attempting to iterate over `fileChangeNodes` and it is `undefined`.
- Fix typo when no git history is available
- Simplify logic in `addCommits`
- Refactor `epoch` time to instead use `ISO` strings

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
vince-fugnitto added a commit that referenced this issue Nov 8, 2018
Fixes #1770
Fixes #1754

- Fixes error thrown when attempting to perform `Date.toLocaleDateString` in the `git-commit-detail-widget`.
- Fixes error thrown when attempting to iterate over `fileChangeNodes` and it is `undefined`.
- Simplify logic in `addCommits`
- Refactor `epoch` time to instead use `ISO` strings

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
kittaakos pushed a commit that referenced this issue Nov 8, 2018
Fixes #1770
Fixes #1754

- Fixes error thrown when attempting to perform `Date.toLocaleDateString` in the `git-commit-detail-widget`.
- Fixes error thrown when attempting to iterate over `fileChangeNodes` and it is `undefined`.
- Simplify logic in `addCommits`
- Refactor `epoch` time to instead use `ISO` strings

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
bogthe pushed a commit to ARMmbed/theia that referenced this issue Jan 21, 2019
Fixes eclipse-theia#1770
Fixes eclipse-theia#1754

- Fixes error thrown when attempting to perform `Date.toLocaleDateString` in the `git-commit-detail-widget`.
- Fixes error thrown when attempting to iterate over `fileChangeNodes` and it is `undefined`.
- Simplify logic in `addCommits`
- Refactor `epoch` time to instead use `ISO` strings

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginners issues that are perfect for beginners bug bugs found in the application git issues related to git
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants