Detect Reporter.select shortcut collisions
Always format files when running `lint --review`
Clean up decision string generation
Add actions for suppression and fixing of all files with the diagnostic
Add extra field to DiagnosticAdviceAction to hide action behind a "See more" option
MODIFY
packages/@romejs/cli-reporter/select.ts
packages/@romejs/cli-reporter/select.ts
@@ -28,7 +28,27 @@ export default async function select<Options extends SelectOptions>(
yes = false,
}: SelectArguments<Options>,
): Promise<Set<keyof Options>> {
- const optionNames: Array<keyof Options> = Object.keys(options);
+ const optionNames: Array<keyof Options> = [];
+ const seenShortcuts: Set<string> = new Set();
+
+ // Verify there's no shortcut collisions and remove empty options
+ for (const key in options) {
+ const option: undefined | SelectOption = options[key];
+
+ if (option !== undefined) {
+ optionNames.push(key);
+
+ const {shortcut} = option;
+ if (shortcut !== undefined) {
+ if (seenShortcuts.has(shortcut)) {
+ throw new Error(`Multiple options have the shortcut ${shortcut}`);
+ } else {
+ seenShortcuts.add(shortcut);
+ }
+ }
+ }
+ }
+
let optionCount = optionNames.length;
if (optionCount === 0) {
return new Set();
@@ -80,7 +100,7 @@ export default async function select<Options extends SelectOptions>(
const optionNames = Object.keys(options);
for (let i = 0; i < optionNames.length; i++) {
const key = optionNames[i];
- const option = options[key];
+ const option = options[key]!;
const {label} = option;
const shortcut = formatShortcut(option);
@@ -152,7 +172,7 @@ export default async function select<Options extends SelectOptions>(
prompt += ': ';
if (selectedOptions.size > 0) {
- prompt += Array.from(selectedOptions, (key) => options[key].label).join(
+ prompt += Array.from(selectedOptions, (key) => options[key]!.label).join(
', ',
);
} else {
@@ -172,7 +192,12 @@ export default async function select<Options extends SelectOptions>(
// Check if this is an option shortcut
if (!key.ctrl) {
for (const optionName in options) {
- const {shortcut} = options[optionName];
+ const option: undefined | SelectOption = options[optionName];
+ if (option === undefined) {
+ continue;
+ }
+
+ const {shortcut} = option;
if (shortcut === key.name) {
if (radio) {
selectedOptions.clear();

MODIFY
packages/@romejs/cli-reporter/types.ts
packages/@romejs/cli-reporter/types.ts
@@ -13,10 +13,10 @@ export type SelectOption = {
};
export type SelectOptions = {
- [key: string]: SelectOption;
+ [key: string]: undefined | SelectOption;
};
-export type SelectArguments<Options> = {
+export type SelectArguments<Options extends SelectOptions> = {
options: Options;
defaults?: Array<keyof Options>;
radio?: boolean;

MODIFY
packages/@romejs/core/client/review.ts
packages/@romejs/core/client/review.ts
@@ -19,6 +19,7 @@ import {Dict} from '@romejs/typescript-helpers';
import {EMPTY_SUCCESS_RESPONSE} from '../master/MasterRequest';
type State = {
+ initial: boolean;
seen: Set<string>;
resolvedCount: number;
};
@@ -27,8 +28,16 @@ async function check(
req: ClientRequest,
state: State,
): Promise<MasterQueryResponse> {
- const {client} = req;
- const {reporter} = client;
+ const {reporter} = req.client;
+
+ reporter.clearScreen();
+
+ if (state.initial) {
+ reporter.info('Fetching initial diagnostics');
+ state.initial = false;
+ } else {
+ reporter.info('Updating diagnostics');
+ }
const res = await req.fork({
...req.query,
@@ -61,11 +70,30 @@ async function check(
return res;
}
+ return await ask(diag, req, state, false);
+}
+
+async function ask(
+ diag: Diagnostic,
+ req: ClientRequest,
+ state: State,
+ more: boolean,
+): Promise<MasterQueryResponse> {
+ const {client} = req;
+ const {reporter} = client;
+ reporter.clearScreen();
+
// Extract actions and remove them from the diagnostic
let {advice = []} = diag.description;
+ let hasExtraOptions = false;
const actions: Array<DiagnosticAdviceAction> = [];
for (const item of advice) {
if (item.type === 'action') {
+ if (item.extra === true && !more) {
+ hasExtraOptions = true;
+ continue;
+ }
+
actions.push(item);
}
}
@@ -100,6 +128,7 @@ async function check(
const options: {
ignore: SelectOption;
exit: SelectOption;
+ more?: SelectOption;
} = {
ignore: {
label: 'Do nothing',
@@ -112,7 +141,13 @@ async function check(
},
};
- reporter.clearScreen();
+ if (!more && hasExtraOptions) {
+ options.more = {
+ label: 'More options...',
+ shortcut: 'm',
+ };
+ }
+
printDiagnostics({
diagnostics: [diag],
suppressions: [],
@@ -129,6 +164,10 @@ async function check(
},
);
+ if (answer === 'more') {
+ return await ask(diag, req, state, true);
+ }
+
if (answer === 'ignore') {
return await check(req, state);
}
@@ -169,15 +208,17 @@ export default async function review(
): Promise<MasterQueryResponse> {
const {reporter} = req.client;
const state: State = {
+ initial: true,
seen: new Set(),
resolvedCount: 0,
};
const res = await check(req, state);
+ reporter.clearScreen();
+
if (state.seen.size === 0) {
reporter.success('Nothing to review!');
} else {
- reporter.clearScreen();
if (res.type === 'DIAGNOSTICS') {
printDiagnostics({
diagnostics: res.diagnostics,

MODIFY
packages/@romejs/core/master/linter/Linter.ts
packages/@romejs/core/master/linter/Linter.ts
@@ -170,7 +170,7 @@ class LintRunner {
hasDecisions,
} = this.options;
const shouldSave = this.linter.shouldSave();
- const shouldApplyFixes = this.linter.shouldApplyFixes();
+ const shouldApplyFixes = !this.linter.shouldOnlyFormat();
const queue: WorkerQueue<void> = new WorkerQueue(master);
@@ -412,14 +412,15 @@ export default class Linter {
request: MasterRequest;
options: LinterOptions;
- shouldApplyFixes(): boolean {
+ shouldOnlyFormat(): boolean {
const {formatOnly} = this.options;
- return !formatOnly;
+ const {review} = this.request.query.requestFlags;
+ return formatOnly || review;
}
shouldSave(): boolean {
- const {save, hasDecisions, formatOnly} = this.options;
- return save || hasDecisions || formatOnly;
+ const {save, hasDecisions} = this.options;
+ return save || hasDecisions || this.shouldOnlyFormat();
}
getFileArgOptions(): MasterRequestGetFilesOptions {

MODIFY
packages/@romejs/diagnostics/descriptions.ts
packages/@romejs/diagnostics/descriptions.ts
@@ -291,7 +291,7 @@ export const descriptions = createMessages({
formatted: string,
) => ({
category: 'lint/pendingFixes',
- message: 'Pending autofixes and formatting',
+ message: 'Pending formatting and recommended autofixes',
advice: [
{
type: 'diff',
@@ -299,23 +299,25 @@ export const descriptions = createMessages({
},
({
type: 'action',
- hidden: true,
command: 'lint',
- instruction: 'To format this file without any fixes run',
- noun: 'Format file',
+ shortcut: 'f',
+ instruction: 'To apply fixes and formatting run',
+ noun: 'Apply fixes and format',
args: [relativeFilename],
commandFlags: {
- formatOnly: true,
+ save: true,
},
} as DiagnosticAdviceAction),
({
type: 'action',
+ hidden: true,
command: 'lint',
- instruction: 'To format and apply recommended fixes run',
- noun: 'Fix file',
+ shortcut: 'o',
+ instruction: 'To format this file without any fixes run',
+ noun: 'Only format',
args: [relativeFilename],
commandFlags: {
- save: true,
+ format: true,
},
} as DiagnosticAdviceAction),
],

MODIFY
packages/@romejs/diagnostics/types.ts
packages/@romejs/diagnostics/types.ts
@@ -131,6 +131,7 @@ export type DiagnosticAdviceInspect = {
export type DiagnosticAdviceAction = {
type: 'action';
hidden?: boolean;
+ extra?: boolean;
shortcut?: string;
instruction: string;
noun: string;

MODIFY
packages/@romejs/js-compiler/lib/CompilerContext.ts
packages/@romejs/js-compiler/lib/CompilerContext.ts
@@ -55,6 +55,8 @@ import {FileReference} from '@romejs/core';
import {DEFAULT_PROJECT_CONFIG} from '@romejs/project';
import {
buildLintDecisionAdviceAction,
+ buildLintDecisionGlobalString,
+ buildLintDecisionString,
deriveDecisionPositionKey,
} from '../lint/decisions';
@@ -316,16 +318,27 @@ export default class CompilerContext {
} else {
advice.push(
buildLintDecisionAdviceAction({
- noun: 'Apply fix',
- instruction: 'To apply this fix run',
filename: this.displayFilename,
+ decision: buildLintDecisionString({
action: 'fix',
+ filename: this.displayFilename,
category,
start: loc.start,
}),
+ shortcut: 'f',
+ noun: 'Apply fix',
+ instruction: 'To apply this fix run',
+ }),
);
- // TODO add fix all with this category option
+ advice.push(
+ buildLintDecisionAdviceAction({
+ extra: true,
+ noun: 'Apply fix for ALL files with this category',
+ instruction: 'To apply fix for ALL files with this category run',
+ decision: buildLintDecisionGlobalString('fix', category),
+ }),
+ );
}
}
@@ -390,12 +403,15 @@ export default class CompilerContext {
: `Apply suggested fix "${suggestion.title}"`,
shortcut: String(num),
instruction: 'To apply this fix run',
+ filename: this.displayFilename,
+ decision: buildLintDecisionString({
filename: this.displayFilename,
action: 'fix',
category,
start: loc.start,
id: index,
}),
+ }),
);
}
@@ -448,14 +464,24 @@ export default class CompilerContext {
noun: 'Add suppression comment',
shortcut: 's',
instruction: 'To suppress this error run',
+ filename: this.displayFilename,
+ decision: buildLintDecisionString({
filename: this.displayFilename,
action: 'suppress',
category,
start: loc.start,
}),
+ }),
);
- // TODO add suppress all action
+ advice.push(
+ buildLintDecisionAdviceAction({
+ extra: true,
+ noun: 'Add suppression comments for ALL files with this category',
+ instruction: 'To add suppression comments for ALL files with this category run',
+ decision: buildLintDecisionGlobalString('suppress', category),
+ }),
+ );
}
const {marker, ...diag} = contextDiag;

MODIFY
packages/@romejs/js-compiler/lint/decisions.ts
packages/@romejs/js-compiler/lint/decisions.ts
@@ -70,7 +70,7 @@ export function parseDecisionStrings(
const globalDecisions: Array<LintCompilerOptionsDecision> = [];
function parseGlobalDecision(parts: Array<string>, i: number) {
- if (parts.length < 3) {
+ if (parts.length !== 3) {
unexpected(descriptions.LINT_COMMAND.INVALID_DECISION_PART_COUNT(i));
}
@@ -86,7 +86,7 @@ export function parseDecisionStrings(
}
function parseLineDecision(parts: Array<string>, i: number) {
- if (parts.length < 4) {
+ if (parts.length < 4 || parts.length > 5) {
unexpected(descriptions.LINT_COMMAND.INVALID_DECISION_PART_COUNT(i));
}
@@ -128,7 +128,7 @@ export function parseDecisionStrings(
const parts = escapeSplit(segment, '-');
if (parts[0] === 'global') {
- parseGlobalDecision(parts, i);
+ parseGlobalDecision(parts.slice(1), i);
} else {
parseLineDecision(parts, i);
}
@@ -137,29 +137,33 @@ export function parseDecisionStrings(
return {lintCompilerOptionsPerFile, globalDecisions};
}
-export function buildLintDecisionAdviceAction(
+function escapeFilename(filename: string): string {
+ return filename.replace(/-/, '\\-');
+}
+
+export function buildLintDecisionGlobalString(
+ action: LintCompilerOptionsDecisionAction,
+ category: DiagnosticCategory,
+): string {
+ return `global-${action}-${category}`;
+}
+
+export function buildLintDecisionString(
{
- noun,
- instruction,
filename,
action,
category,
start,
- shortcut,
id,
}: {
- shortcut?: string;
- noun: string;
- instruction: string;
filename: string;
- action: LintCompilerOptionsDecision['action'];
+ action: LintCompilerOptionsDecisionAction;
category: DiagnosticCategory;
start: Position;
id?: number;
},
-): DiagnosticAdviceAction {
- const escapedFilename = filename.replace(/-/, '\\-');
-
+): string {
+ const escapedFilename = escapeFilename(filename);
const pos = deriveDecisionPositionKey(action, {start});
const parts = [action, category, escapedFilename, pos];
@@ -168,14 +172,33 @@ export function buildLintDecisionAdviceAction(
parts.push(String(id));
}
- const decision = parts.join('-');
+ return parts.join('-');
+}
+export function buildLintDecisionAdviceAction(
+ {
+ noun,
+ instruction,
+ filename,
+ shortcut,
+ decision,
+ extra,
+ }: {
+ extra?: boolean;
+ shortcut?: string;
+ noun: string;
+ instruction: string;
+ filename?: string;
+ decision: string;
+ },
+): DiagnosticAdviceAction {
return {
type: 'action',
+ extra,
hidden: true,
command: 'lint',
shortcut,
- args: [filename],
+ args: filename === undefined ? [] : [escapeFilename(filename)],
noun,
instruction,
commandFlags: {

- Detect Reporter.select shortcut collisions
- Always format files when running `lint --review`
- Clean up decision string generation
- Add actions for suppression and fixing of all files with the diagnostic
- Add extra field to DiagnosticAdviceAction to hide action behind a "See more" option