Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions extensions/ql-vscode/src/pure/time.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Contains an assortment of helper functions for working with time, dates, and durations.
*/


const durationFormatter = new Intl.RelativeTimeFormat('en', {
numeric: 'auto',
});

// All these are approximate, specifically months and years
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit

Suggested change
// All these are approximate, specifically months and years
// Months and years are approximate

const MINUTE_IN_MILLIS = 1000 * 60;
const HOUR_IN_MILLIS = 60 * MINUTE_IN_MILLIS;
const DAY_IN_MILLIS = 24 * HOUR_IN_MILLIS;
const MONTH_IN_MILLIS = 30 * DAY_IN_MILLIS;
const YEAR_IN_MILLIS = 365 * DAY_IN_MILLIS;

export function humanizeDuration(diffInMs?: number) {
Comment thread
charisk marked this conversation as resolved.
Outdated
if (diffInMs === undefined) {
return '';
}

if (Math.abs(diffInMs) < HOUR_IN_MILLIS) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we be covering seconds as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It didn't seem to make sense for our purposes.

return durationFormatter.format(Math.floor(diffInMs / MINUTE_IN_MILLIS), 'minute');
} else if (Math.abs(diffInMs) < DAY_IN_MILLIS) {
return durationFormatter.format(Math.floor(diffInMs / HOUR_IN_MILLIS), 'hour');
} else if (Math.abs(diffInMs) < MONTH_IN_MILLIS) {
return durationFormatter.format(Math.floor(diffInMs / DAY_IN_MILLIS), 'day');
} else if (Math.abs(diffInMs) < YEAR_IN_MILLIS) {
return durationFormatter.format(Math.floor(diffInMs / MONTH_IN_MILLIS), 'month');
} else {
return durationFormatter.format(Math.floor(diffInMs / YEAR_IN_MILLIS), 'year');
}
}

function createFormatter(unit: string) {
return Intl.NumberFormat('en-US', {
style: 'unit',
unit,
unitDisplay: 'long'
});
}

export function humanizeUnit(diffInMs?: number): string {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have a very tired brain, but I can't easily understand the difference between humanizeDuration and humanizeUnit - can we think of different names?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll add comments. I can't think of better names.

// assume a blank or empty string is a zero
// assume anything less than 0 is a zero
if (!diffInMs || diffInMs < MINUTE_IN_MILLIS) {
return 'Less than a minute';
}
let unit: string;
let unitDiff: number;
if (diffInMs < HOUR_IN_MILLIS) {
unit = 'minute';
unitDiff = Math.floor(diffInMs / MINUTE_IN_MILLIS);
} else if (diffInMs < DAY_IN_MILLIS) {
unit = 'hour';
unitDiff = Math.floor(diffInMs / HOUR_IN_MILLIS);
} else if (diffInMs < MONTH_IN_MILLIS) {
unit = 'day';
unitDiff = Math.floor(diffInMs / DAY_IN_MILLIS);
} else if (diffInMs < YEAR_IN_MILLIS) {
unit = 'month';
unitDiff = Math.floor(diffInMs / MONTH_IN_MILLIS);
} else {
unit = 'year';
unitDiff = Math.floor(diffInMs / YEAR_IN_MILLIS);
}

return createFormatter(unit).format(unitDiff);
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { URLSearchParams } from 'url';
import { SHOW_QUERY_TEXT_MSG } from '../query-history';
import { AnalysesResultsManager } from './analyses-results-manager';
import { AnalysisResults } from './shared/analysis-result';
import { humanizeUnit } from '../pure/time';

export class RemoteQueriesInterfaceManager {
private panel: WebviewPanel | undefined;
Expand Down Expand Up @@ -249,23 +250,7 @@ export class RemoteQueriesInterfaceManager {

private getDuration(startTime: number, endTime: number): string {
const diffInMs = startTime - endTime;
return this.formatDuration(diffInMs);
}

private formatDuration(ms: number): string {
const seconds = ms / 1000;
const minutes = seconds / 60;
const hours = minutes / 60;
const days = hours / 24;
if (days > 1) {
return `${days.toFixed(2)} days`;
} else if (hours > 1) {
return `${hours.toFixed(2)} hours`;
} else if (minutes > 1) {
return `${minutes.toFixed(2)} minutes`;
} else {
return `${seconds.toFixed(2)} seconds`;
}
return humanizeUnit(diffInMs);
}

private formatDate = (millis: number): string => {
Expand Down
32 changes: 3 additions & 29 deletions extensions/ql-vscode/src/remote-queries/view/LastUpdated.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import * as React from 'react';
import { RepoPushIcon } from '@primer/octicons-react';
import styled from 'styled-components';

import { humanizeDuration } from '../../pure/time';

const IconContainer = styled.span`
flex-grow: 0;
text-align: right;
Expand All @@ -23,7 +25,7 @@ const LastUpdated = ({ lastUpdated }: Props) => (
<RepoPushIcon size={16} />
</IconContainer>
<Duration>
{humanizeDuration(lastUpdated)}
{humanizeDuration(lastUpdated && -lastUpdated)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand this line of code — could you explain it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was about to ask the same question! What does it mean to && these two numbers? 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

lastUpdated may be undefined. If so, do not want to negate it (this would convert it to NaN, which is BAD).

If lastUpdated is 0, then negating is a noop. So we don't need to negate.

Would it be easier to understand if I did this?


lastUpdated === undefined ? undefined : -lastUpdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
{humanizeDuration(lastUpdated)}
{humanizeDuration(lastUpdated && -lastUpdated)}
{humanizeDuration(lastUpdated === undefined ? undefined : -lastUpdated)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hm, yes, I find that easier to understand. What happens if lastUpdated is an integer other than 0 though? Do we expect it to be a positive or negative integer (and why do we need to negate it at all if the absolute value is taken in humanizeDuration()?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good questions. If lastUpdated is something other than 0, then it is negated (meaning, a time in the past).

Absolute value is used to determine which unit to use. A positive integer indicates "XXX units from now". A negative integer indicates "XXX units ago".

Maybe it would be clearer if we ensured that lastUpdated was always negative and I can put a comment there.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh yes! I see how it works with the new change now. I think that would make it clearer, yes!

</Duration>
</>
) : (
Expand All @@ -32,31 +34,3 @@ const LastUpdated = ({ lastUpdated }: Props) => (
);

export default LastUpdated;

const formatter = new Intl.RelativeTimeFormat('en', {
numeric: 'auto'
});

// All these are approximate, specifically months and years
const MINUTE_IN_MILLIS = 1000 * 60;
const HOUR_IN_MILLIS = 60 * MINUTE_IN_MILLIS;
const DAY_IN_MILLIS = 24 * HOUR_IN_MILLIS;
const MONTH_IN_MILLIS = 30 * DAY_IN_MILLIS;
const YEAR_IN_MILLIS = 365 * DAY_IN_MILLIS;

function humanizeDuration(diff?: number) {
if (!diff) {
return '';
}
if (diff < HOUR_IN_MILLIS) {
return formatter.format(- Math.floor(diff / MINUTE_IN_MILLIS), 'minute');
} else if (diff < DAY_IN_MILLIS) {
return formatter.format(- Math.floor(diff / HOUR_IN_MILLIS), 'hour');
} else if (diff < MONTH_IN_MILLIS) {
return formatter.format(- Math.floor(diff / DAY_IN_MILLIS), 'day');
} else if (diff < YEAR_IN_MILLIS) {
return formatter.format(- Math.floor(diff / MONTH_IN_MILLIS), 'month');
} else {
return formatter.format(- Math.floor(diff / YEAR_IN_MILLIS), 'year');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,19 @@ const openQueryTextVirtualFile = (queryResult: RemoteQueryResult) => {
});
};

function createResultsDescription(queryResult: RemoteQueryResult) {
return `${queryResult.totalResultCount} results from running against ${queryResult.totalRepositoryCount
Comment thread
charisk marked this conversation as resolved.
Outdated
} ${queryResult.totalRepositoryCount === 1 ? 'repository' : 'repositories'
} (${queryResult.executionDuration}), ${queryResult.executionTimestamp}`;
}

const sumAnalysesResults = (analysesResults: AnalysisResults[]) =>
analysesResults.reduce((acc, curr) => acc + getAnalysisResultCount(curr), 0);

const QueryInfo = (queryResult: RemoteQueryResult) => (
<>
<VerticalSpace size={1} />
{queryResult.totalResultCount} results from running against {queryResult.totalRepositoryCount} repositories
({queryResult.executionDuration}), {queryResult.executionTimestamp}
{createResultsDescription(queryResult)}
<VerticalSpace size={1} />
<span>
<a className="vscode-codeql__query-info-link" href="#" onClick={() => openQueryFile(queryResult)}>
Expand Down
73 changes: 73 additions & 0 deletions extensions/ql-vscode/test/pure-tests/time.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { expect } from 'chai';
import 'mocha';

import { humanizeDuration, humanizeUnit } from '../../src/pure/time';

describe('Time', () => {
it('should return a humanized unit', () => {
expect(humanizeUnit(undefined)).to.eq('Less than a minute');
expect(humanizeUnit(0)).to.eq('Less than a minute');
expect(humanizeUnit(-1)).to.eq('Less than a minute');
expect(humanizeUnit(1000 * 60 - 1)).to.eq('Less than a minute');
expect(humanizeUnit(1000 * 60)).to.eq('1 minute');
expect(humanizeUnit(1000 * 60 * 2 - 1)).to.eq('1 minute');
expect(humanizeUnit(1000 * 60 * 2)).to.eq('2 minutes');
expect(humanizeUnit(1000 * 60 * 60)).to.eq('1 hour');
expect(humanizeUnit(1000 * 60 * 60 * 2)).to.eq('2 hours');
expect(humanizeUnit(1000 * 60 * 60 * 24)).to.eq('1 day');
expect(humanizeUnit(1000 * 60 * 60 * 24 * 2)).to.eq('2 days');

// assume every month has 30 days
expect(humanizeUnit(1000 * 60 * 60 * 24 * 30)).to.eq('1 month');
expect(humanizeUnit(1000 * 60 * 60 * 24 * 30 * 2)).to.eq('2 months');
expect(humanizeUnit(1000 * 60 * 60 * 24 * 30 * 12)).to.eq('12 months');

// assume every year has 365 days
expect(humanizeUnit(1000 * 60 * 60 * 24 * 365)).to.eq('1 year');
expect(humanizeUnit(1000 * 60 * 60 * 24 * 365 * 2)).to.eq('2 years');
});

it('should return a humanized duration positive', () => {
expect(humanizeDuration(undefined)).to.eq('');
expect(humanizeDuration(0)).to.eq('this minute');
expect(humanizeDuration(1)).to.eq('this minute');
expect(humanizeDuration(1000 * 60 - 1)).to.eq('this minute');
expect(humanizeDuration(1000 * 60)).to.eq('in 1 minute');
expect(humanizeDuration(1000 * 60 * 2 - 1)).to.eq('in 1 minute');
expect(humanizeDuration(1000 * 60 * 2)).to.eq('in 2 minutes');
expect(humanizeDuration(1000 * 60 * 60)).to.eq('in 1 hour');
expect(humanizeDuration(1000 * 60 * 60 * 2)).to.eq('in 2 hours');
expect(humanizeDuration(1000 * 60 * 60 * 24)).to.eq('tomorrow');
expect(humanizeDuration(1000 * 60 * 60 * 24 * 2)).to.eq('in 2 days');

// assume every month has 30 days
expect(humanizeDuration(1000 * 60 * 60 * 24 * 30)).to.eq('next month');
expect(humanizeDuration(1000 * 60 * 60 * 24 * 30 * 2)).to.eq('in 2 months');
expect(humanizeDuration(1000 * 60 * 60 * 24 * 30 * 12)).to.eq('in 12 months');

// assume every year has 365 days
expect(humanizeDuration(1000 * 60 * 60 * 24 * 365)).to.eq('next year');
expect(humanizeDuration(1000 * 60 * 60 * 24 * 365 * 2)).to.eq('in 2 years');
});

it('should return a humanized duration negative', () => {
expect(humanizeDuration(-1)).to.eq('1 minute ago');
expect(humanizeDuration(-1000 * 60)).to.eq('1 minute ago');
expect(humanizeDuration(-1000 * 60 - 1)).to.eq('2 minutes ago');
expect(humanizeDuration(-1000 * 60 * 2)).to.eq('2 minutes ago');
expect(humanizeDuration(-1000 * 60 * 2 - 1)).to.eq('3 minutes ago');
expect(humanizeDuration(-1000 * 60 * 60)).to.eq('1 hour ago');
expect(humanizeDuration(-1000 * 60 * 60 * 2)).to.eq('2 hours ago');
expect(humanizeDuration(-1000 * 60 * 60 * 24)).to.eq('yesterday');
expect(humanizeDuration(-1000 * 60 * 60 * 24 * 2)).to.eq('2 days ago');

// assume every month has 30 days
expect(humanizeDuration(-1000 * 60 * 60 * 24 * 30)).to.eq('last month');
expect(humanizeDuration(-1000 * 60 * 60 * 24 * 30 * 2)).to.eq('2 months ago');
expect(humanizeDuration(-1000 * 60 * 60 * 24 * 30 * 12)).to.eq('12 months ago');

// assume every year has 365 days
expect(humanizeDuration(-1000 * 60 * 60 * 24 * 365)).to.eq('last year');
expect(humanizeDuration(-1000 * 60 * 60 * 24 * 365 * 2)).to.eq('2 years ago');
});
});