feat: improve task naming

[jd20]

A couple improvements to task naming:

• For function commands, search for first occurrence of a filename argument, and strip from that point onwards.
• For commands in general, strip common package runner prefixes (and their options): npm, npx, yarn (unless it would strip the entire command).
• Drop the ellipsis from function commands, to be consistent with non-function commands.

Fixes: #828

[iiroj]

Not sure about this. How will it format something like

(files) => `npm run test -- --update ${files.join(' ')}`

[jd20]

It'll format it as run test -- --update. Do you think it makes sense to special-case the run command? Or maybe in general it is going to be too difficult to properly handle package runner commands, without overcomplicating things?

[iiroj]

I think it’s best to leave out stripping those. I’d imagine there’s always some reason to run via npm/yarn instead of directly, so better not hide it.

[jd20]

Sounds good, I'll update the PR.

[iiroj]

Looks nice, and a good improvement!

Let's just leave out the second bullet of stripping out prefixes to simplify code and not cause unexpected issues (users not recognizing the command).

[iiroj]

It seems the path match doesn't quite work on Windows:

- LOG [Function] prettier --write [started]
- LOG [Function] prettier --write [completed]
+ LOG [Function] prettier --write C:\projects\lint-staged-test\AqocQtX4BMGjgyZNYR1jx\test.js [started]
+ LOG [Function] prettier --write C:\projects\lint-staged-test\AqocQtX4BMGjgyZNYR1jx\test.js [completed]

Appveyor logs

[iiroj]

I fear this might not work as-is, since the task function can format files.

It might be necessary to do something like:

const fileBasenames = files.map(file => path.basename(file))
const indexOfFile = words.findIndex(word => fileBasenames.some(basename => word.includes(basename)))

to match only the basename (ie. file.js).

This is bound to have performance indications, however.

[jd20]

Yeah, I also realized tokenizing by splitting on space isn't going to work properly either, if there are spaces inside the filename. Let me revise it a bit more, to try and handle the basename logic above. Is the function formatting files the reason Windows test is failing as well?

[iiroj]

Can’t say for sure, though I would think the files available in this method are normalized the same way; apparently not.

[iiroj]

Maybe the simplest method would be to, for the string npm run test -- -u src/staged.js, to do:

1. Get the os path separator from path.sep. Eg. /
2. Split the command string from this and get the first part, eg npm run test -- -u src
3. Split the above from the last space and get the first part, eg npm run test -- -u

It should also be try/catched to a short fallback if no path.sep is not found, so that the entire string would not end up in the console.

What do you think?

[jd20]

Maybe the simplest method would be to, for the string npm run test -- -u src/staged.js, to do:

1. Get the os path separator from path.sep. Eg. /
2. Split the command string from this and get the first part, eg npm run test -- -u src
3. Split the above from the last space and get the first part, eg npm run test -- -u

It should also be try/catched to a short fallback if no path.sep is not found, so that the entire string would not end up in the console.

What do you think?

Wouldn't that fail for commands with no path separators (staged files in same directory), as well as potentially strip out starting from a non-staged file, such as config?

I'm thinking to tokenize the command (taking quotes into account), then walk through each token, checking for the first one that matches to the staged set (can resolve and compare absolute paths, if we want to be certain it's same file). Then strip from that point onwards. I can optimize the check against the staged set, if we're concerned about perf.

[iiroj]

@jd20 You are correct about the failing!

[fantapop]

@iiroj I'm interested in this as well. I'm Wondering if something simpler like being about to put in a manual override would be better. I'm also wondering why the command is getting cut off in so few characters? It seems like it could afford to go up to the width of the terminal.

[iiroj]

@fantapop thanks for the terminal width suggestion! That seems like a pretty easy improvement.

EDIT: Custom names would also be possible, via a static method for example. This would be pretty advanced though:

/** Run eslint –-fix for all JSX files */
const fixJsxFilesTask = (files) =>
  `eslint --fix {files.filter(file => file.endsWith('.jsx')).join(' ')}`
/** Add custom name via the toString method */
fixJsxFilesTask.toString = (files) =>
  `Fix JSX files (matched ${files.length} files)`

module.exports = {
  '*': fixJsxFilesTask
}

[iiroj]

I opened PR #865 which will always take the full length task title (for both "simple" and functional tasks), and then truncates it to the current terminal columns, minus the prefix padding length caused by listr2. This means the task title will be as long as possible to fit on a single line, and will be clipped with an ellipsis ….

I'll close this PR since that should be a good enough solution.

[fantapop]

This solution is working great. Thank you!