Skip to content

Commit

Permalink
Don’t print full commands in non-interactive mode
Browse files Browse the repository at this point in the history
  • Loading branch information
@lydell
lydell committed Aug 20, 2022
1 parent 0d609a3 commit d603dbc
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 69 deletions.
42 changes: 31 additions & 11 deletions run-pty.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ const drawDashboardCommandLines = (
label: shortcut(label, { pad: false }),
icon,
status,
title: command.titleWithGraphicRenditions,
title: command.titlePossiblyWithGraphicRenditions,
};
});

Expand Down Expand Up @@ -485,7 +485,7 @@ const drawSummary = (commands) => {
});
return `${indicator}${EMOJI_WIDTH_FIX} ${
status === undefined ? "" : `${status} `
}${command.formattedCommandWithTitle}${RESET_COLOR}`;
}${command.titlePossiblyWithGraphicRenditions}${RESET_COLOR}`;
});
return `${bold(`Summary – ${summary}:`)}\n${lines.join("\n")}\n`;
};
Expand Down Expand Up @@ -519,15 +519,15 @@ const getPid = (command) =>
: "";

/**
* @typedef {Pick<Command, "formattedCommandWithTitle" | "title" | "cwd" | "history">} CommandText
* @typedef {Pick<Command, "formattedCommandWithTitle" | "title" | "titlePossiblyWithGraphicRenditions" | "cwd" | "history">} CommandText
*/

/**
* @param {CommandText} command
* @returns {string}
*/
const cwdText = (command) =>
path.resolve(command.cwd) === process.cwd() || command.cwd === command.title
path.resolve(command.cwd) === process.cwd()
? ""
: `${folder}${EMOJI_WIDTH_FIX} ${dim(command.cwd)}\n`;

Expand All @@ -540,13 +540,31 @@ const historyStart = (indicator, command) =>
`${commandTitleWithIndicator(indicator, command)}\n${cwdText(command)}`;

/**
* Used in interactive mode.
*
* @param {string} indicator
* @param {CommandText} command
* @returns {string}
*/
const commandTitleWithIndicator = (indicator, command) =>
`${indicator}${EMOJI_WIDTH_FIX} ${command.formattedCommandWithTitle}${RESET_COLOR}`;

/**
* Similar to `commandTitleWithIndicator`. Used in non-interactive mode. This
* does not print the full command, only the title. In interactive mode, the
* dashboard only prints the title too – if you want the full thing, you need to
* enter that command, because the command can be very long. In non-interactive
* mode, it can be very spammy if a long command is printed once when started,
* once when exited and once in the summary – interesting output (such as
* errors) gets lost in a sea of command stuff.
*
* @param {string} indicator
* @param {CommandText} command
* @returns {string}
*/
const commandTitleOnlyWithIndicator = (indicator, command) =>
`${indicator}${EMOJI_WIDTH_FIX} ${command.titlePossiblyWithGraphicRenditions}${RESET_COLOR}`;

/**
* @param {Array<Command>} commands
* @returns {string}
Expand Down Expand Up @@ -610,7 +628,7 @@ const exitTextAndHistory = ({ command, exitCode, numExited, numTotal }) => {
// If the last line is empty, no extra newline is needed.
lastLine.trim() === "" ? "" : "\n";
return `
${commandTitleWithIndicator(exitIndicator(exitCode), command)}
${commandTitleOnlyWithIndicator(exitIndicator(exitCode), command)}
${cwdText(command)}${command.history}${newline}${bold(
`exit ${exitCode}`
)} ${dim(`(${numExited}/${numTotal} exited)`)}
Expand Down Expand Up @@ -1070,13 +1088,15 @@ class Command {
this.cwd = cwd;
this.killAllSequence = killAllSequence;
this.title = removeGraphicRenditions(title);
this.titleWithGraphicRenditions = title;
this.titlePossiblyWithGraphicRenditions = NO_COLOR
? removeGraphicRenditions(title)
: title;
this.formattedCommandWithTitle =
title === formattedCommand
? formattedCommand
: NO_COLOR
? `${removeGraphicRenditions(title)}: ${formattedCommand}`
: `${bold(`${title}${RESET_COLOR}:`)} ${formattedCommand}`;
: `${bold(`${title}`)}: ${formattedCommand}`;
this.onData = onData;
this.onRequest = onRequest;
this.onExit = onExit;
Expand Down Expand Up @@ -2012,7 +2032,7 @@ const runNonInteractively = (commandDescriptions, maxParallel) => {
for (const command of notExited) {
command.kill();
process.stdout.write(
`${commandTitleWithIndicator(killingIndicator, command)}\n\n`
`${commandTitleOnlyWithIndicator(killingIndicator, command)}\n\n`
);
}
}
Expand Down Expand Up @@ -2073,7 +2093,7 @@ const runNonInteractively = (commandDescriptions, maxParallel) => {
const command = commands[nextWaitingIndex];
command.start();
process.stdout.write(
`${commandTitleWithIndicator(runningIndicator, command)}\n\n`
`${commandTitleOnlyWithIndicator(runningIndicator, command)}\n\n`
);
}

Expand Down Expand Up @@ -2101,11 +2121,11 @@ const runNonInteractively = (commandDescriptions, maxParallel) => {
if (index < maxParallel) {
command.start();
process.stdout.write(
`${commandTitleWithIndicator(runningIndicator, command)}\n\n`
`${commandTitleOnlyWithIndicator(runningIndicator, command)}\n\n`
);
} else {
process.stdout.write(
`${commandTitleWithIndicator(waitingIndicator, command)}\n\n`
`${commandTitleOnlyWithIndicator(waitingIndicator, command)}\n\n`
);
}
}
Expand Down
93 changes: 35 additions & 58 deletions test/run-pty.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ function fakeCommand(item, index = 0) {
return {
label: ALL_LABELS[index],
title,
titleWithGraphicRenditions: title,
titlePossiblyWithGraphicRenditions: title,
formattedCommandWithTitle: commandToPresentationName(item.command),
status: item.status,
// Unused in this case:
Expand Down Expand Up @@ -646,13 +646,19 @@ describe("focused command", () => {
/**
* @param {(command: import("../run-pty").CommandText) => string} f
* @param {string} formattedCommandWithTitle
* @param {string} title
* @param {string} cwd
* @returns {string}
*/
function render(f, formattedCommandWithTitle, title, cwd) {
function render(f, formattedCommandWithTitle, cwd) {
return replaceAnsi(
f({ formattedCommandWithTitle, title, cwd, history: "" })
f({
formattedCommandWithTitle,
title: "Expected `title` not to be used",
titlePossiblyWithGraphicRenditions:
"Expected `titlePossiblyWithGraphicRenditions` not to be used",
cwd,
history: "",
})
);
}

Expand All @@ -661,7 +667,6 @@ describe("focused command", () => {
render(
(command) => historyStart(runningIndicator, command),
"npm start",
"npm start",
"./"
)
).toMatchInlineSnapshot(`
Expand All @@ -675,7 +680,6 @@ describe("focused command", () => {
render(
(command) => historyStart(runningIndicator, command),
"frontend: npm start",
"frontend",
"web/frontend"
)
).toMatchInlineSnapshot(`
Expand All @@ -685,20 +689,6 @@ describe("focused command", () => {
`);
});

test("cwd not shown if same as title", () => {
expect(
render(
(command) => historyStart(runningIndicator, command),
"frontend: npm start",
"frontend",
"frontend"
)
).toMatchInlineSnapshot(`
🟢 frontend: npm start⧘␊
`);
});

test("running text includes pid", () => {
expect(replaceAnsi(runningText(12345))).toMatchInlineSnapshot(`
⧙[⧘⧙ctrl+c⧘⧙]⧘ kill ⧙(pid 12345)⧘
Expand All @@ -707,27 +697,16 @@ describe("focused command", () => {
});

test("killing without cwd", () => {
expect(
render(
() => killingText(12345),
"frontend: npm start",
"frontend",
"./x/.."
)
).toMatchInlineSnapshot(`
expect(render(() => killingText(12345), "frontend: npm start", "./x/.."))
.toMatchInlineSnapshot(`
⧙[⧘⧙ctrl+c⧘⧙]⧘ kill ⧙(double-press to force) (pid 12345)⧘
⧙[⧘⧙ctrl+z⧘⧙]⧘ dashboard
`);
});

test("killing with cwd", () => {
expect(
render(
() => killingText(12345),
"frontend: npm start",
"frontend",
"web/frontend"
)
render(() => killingText(12345), "frontend: npm start", "web/frontend")
).toMatchInlineSnapshot(`
⧙[⧘⧙ctrl+c⧘⧙]⧘ kill ⧙(double-press to force) (pid 12345)⧘
⧙[⧘⧙ctrl+z⧘⧙]⧘ dashboard
Expand All @@ -739,7 +718,6 @@ describe("focused command", () => {
render(
(command) => exitText([], command, 0, { tag: "NoAutoExit" }),
"frontend: npm start",
"frontend",
"web/frontend"
)
).toMatchInlineSnapshot(`
Expand All @@ -758,8 +736,7 @@ describe("focused command", () => {
render(
(command) => exitText([], command, 0, { tag: "NoAutoExit" }),
"frontend: npm start",
"frontend",
"frontend"
"."
)
).toMatchInlineSnapshot(`
⚪ frontend: npm start⧘
Expand All @@ -780,8 +757,7 @@ describe("focused command", () => {
maxParallel: 3,
}),
"frontend: npm start",
"frontend",
"frontend"
"."
)
).toMatchInlineSnapshot(`
⚪ frontend: npm start⧘
Expand All @@ -797,8 +773,7 @@ describe("focused command", () => {
render(
(command) => exitText([], command, 1, { tag: "NoAutoExit" }),
"frontend: npm start",
"frontend",
"frontend"
"."
)
).toMatchInlineSnapshot(`
🔴 frontend: npm start⧘
Expand All @@ -811,14 +786,8 @@ describe("focused command", () => {
});

test("waiting text", () => {
expect(
render(
() => waitingText([]),
"frontend: npm start",
"frontend",
"frontend"
)
).toMatchInlineSnapshot(`
expect(render(() => waitingText([]), "frontend: npm start", "."))
.toMatchInlineSnapshot(`
Waiting for other commands to finish before starting.
⧙[⧘⧙ctrl+c⧘⧙]⧘ exit
Expand All @@ -828,15 +797,24 @@ describe("focused command", () => {
});

describe("exit text and history", () => {
/** @type {import("../run-pty").CommandText} */
const command = {
cwd: ".",
formattedCommandWithTitle:
"Expected `formattedCommandWithTitle` not to be used.",
title: "Expected `title` not to be used.",
titlePossiblyWithGraphicRenditions:
"Expected `titlePossiblyWithGraphicRenditions` not to be used.",
history: "",
};

test("one command, no history", () => {
expect(
replaceAnsi(
exitTextAndHistory({
command: {
cwd: ".",
formattedCommandWithTitle: "npm test",
title: "npm test",
history: "",
...command,
titlePossiblyWithGraphicRenditions: "npm test",
},
exitCode: 0,
numExited: 1,
Expand All @@ -856,9 +834,8 @@ describe("exit text and history", () => {
replaceAnsi(
exitTextAndHistory({
command: {
cwd: ".",
formattedCommandWithTitle: "npm test",
title: "npm test",
...command,
titlePossiblyWithGraphicRenditions: "npm test",
history: ["First line", "Second line", ""].join("\n"),
},
exitCode: 1,
Expand All @@ -881,9 +858,9 @@ describe("exit text and history", () => {
replaceAnsi(
exitTextAndHistory({
command: {
...command,
cwd: "web/frontend",
formattedCommandWithTitle: "npm test",
title: "npm test",
titlePossiblyWithGraphicRenditions: "npm test",
history: ["First line", "Second line"].join("\n"),
},

Expand Down

0 comments on commit d603dbc

Please sign in to comment.