-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Change a way of eliding filenames #1437
Conversation
Elide hash in filename, even if the file length doesn't extend maximum. Always present file extension in elided form. Closes: GoogleChrome#1372
f119337
to
ff4c1d4
Compare
}); | ||
|
||
it('Elides long names with hash', () => { | ||
const url = superLongName.slice(0, -3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
templates str would be great here:
`${superLongName.slice(0, -3)}-f303dec6eec305a4fab8025577db3c2feb418148ac75ba378281399fb1ba670b.css`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid with indent and const keyword that would exceed lint line width
@@ -129,3 +129,32 @@ describe('CRC Formatter', () => { | |||
assert.ok(truncatedURLRegExp.test(output)); | |||
}); | |||
}); | |||
|
|||
|
|||
describe('parseURL', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like there are some html tests above. Can you add a test for the title
attr addition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, added
if (file.length > MAX_FILENAME_LENGTH) { | ||
file = file.slice(0, MAX_FILENAME_LENGTH) + '...'; | ||
const dotIndex = file.lastIndexOf('.'); | ||
file = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file = file.slice(0, MAX_FILENAME_LENGTH - 1 - (file.length - dotIndex)) +
`\u2026${file.slice(dotIndex)}`;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
it('Elides hashes', () => { | ||
const url = 'http://example.com/file-f303dec6eec305a4fab8025577db3c2feb418148ac75ba378281399fb1ba670b.css'; | ||
const result = criticalRequestChainFormatter.parseURL(url); | ||
assert.equal(result.file, '/file-f303dec\u2026.css'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a test for a hash in the middle:
url = 'http://example.com/file-f303dec6eec305a4fab80378281399fb1ba670b-somethingmore.css';
result = criticalRequestChainFormatter.parseURL(url);
assert.equal(result.file, '/file-f303dec\u2026-somethingmore.css');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
00f16f4
to
5854941
Compare
@ebidel, thanks for a rapid fast code review! Changes done. |
5854941
to
8487a40
Compare
Thanks @Lokson. Good by me. @paulirish is this what you had in mind? |
You're welcome! |
@Lokson looks good. It probably makes sense to move this logic into |
how much of it though? Just eliding long filenames or also the part about stripping down to the end of the |
both. in devtools source we call it |
maybe for lighthouse a good name would be getShortname? |
Static |
yup! sgtm. nice work @Lokson |
Elide hash in filename, even if the file length doesn't extend maximum. Always present file extension in elided form. Closes: GoogleChrome#1372 * Refactor and add tests * Extract getDisplayName into url-shim
Elide hash in filename even if the file length doesn't extend maximum.
Always present file extension in elided form.
Show full url in the title attribute (it will be seen on mouse hover).
Closes: #1372