Merge pull request #13075 from keymanapp/fix/developer/13072-make-kmc-log-options-consistent

fix(developer): make kmc log options consistent across all commands

Author: mcdurdin

Diff for: developer/docs/help/reference/kmc/cli/reference.md

@@ -90,14 +90,33 @@ The following parameters are available:
 
 : Prints the version number of kmc
 
-`--no-error-reporting`, `--error-reporting`
+`--color`, `--no-color`
+
+: Controls colorization for log messages, using ANSI color controls. If both of
+  these settings are omitted, kmc will attempt to detect from console, and will
+  use colorization for interactive terminals, and no colorization when
+  redirection is being used.
+
+`--error-reporting`, `--no-error-reporting`
 
-: Enable or disable error reporting to keyman.com, overriding [user
-  settings](../../user-settings). Error reporting is for fatal errors in the
-  compiler, and not errors in compiled files. No user data is sent in error
+: Enable or disable error reporting to keyman.com, overriding
+  [user settings](../../user-settings). Error reporting is for fatal errors in
+  the compiler, and not errors in compiled files. No user data is sent in error
   reports, although some filenames and paths may be present in the diagnostic
   data attached to the report.
 
+`--log-format <logFormat>`
+
+: Output log format. The available options are:
+  * `formatted` (default): emits log messages in a human-readable format
+  * `tsv`: emits log messages in UTF-8 tab-separated format. This format will be
+    stable across versions of kmc. The format has the following fields:
+    * filename
+    * line number
+    * severity
+    * code
+    * message
+
 `-l <logLevel>`, `--log-level <logLevel>`
 
 : Controls the level of logging to console for messages relating to the
@@ -108,23 +127,19 @@ The following parameters are available:
   * `hint`: emits compilation errors, warnings, and hints
   * `info` (default): emits compilation errors, warnings, hints, and
     informational messages
+  * `verbose`: emits all compilation messages, including verbose
+    informational messages
   * `debug`: emits all compilation messages, plus internal debug messages
 
-  Logging of specific [messages](messages) can be controlled with `--message`, `-m`.
+  Logging of specific [messages](messages) can be controlled with `--message`,
+  `-m` (`build` command only).
 
   Note that when warnings are treated as errors
   (`--compiler-warnings-as-errors`, `-w`), they will still be logged as
   warnings, so suppressing warnings while using this flag may be confusing.
 
 ## `kmc build` options
 
-`--color`, `--no-color`
-
-: Controls colorization for log messages, using ANSI color controls. If both of
-  these settings are omitted, kmc will attempt to detect from console, and will
-  use colorization for interactive terminals, and no colorization when
-  redirection is being used.
-
 `-d`, `--debug`
 
 : Include debug information in output files. Debug information is used for
@@ -167,17 +182,6 @@ The following parameters are available:
 : Turns off warnings (CWARN_HeaderStatementIsDeprecated,
   CWARN_LanguageHeadersDeprecatedInKeyman10) for deprecated code styles
 
-`--log-format <logFormat>`
-
-: Output log format. The available options are:
-  * `formatted` (default): emits log messages in a human-readable format
-  * `tsv`: emits log messages in UTF-8 tab-separated format. This format will be
-    stable across versions of kmc. The format has the following fields:
-    * filename
-    * line number
-    * severity
-    * code
-    * message
 
 `-o <filename>`, `--out-file <filename>`
 

Diff for: developer/src/common/web/utils/src/compiler-interfaces.ts

@@ -291,10 +291,8 @@ export interface CompilerMessageOverride {
   level: CompilerErrorSeverityOverride;
 };
 
-export interface CompilerCallbackOptions {
-  logLevel?: CompilerLogLevel;
-  logFormat?: CompilerLogFormat;
-  color?: boolean; // null or undefined == use console default
+export interface CompilerCallbackOptions extends CompilerBaseOptions {
+  // TODO: these overlap with CompilerOptions, should refactor
   compilerWarningsAsErrors?: boolean;
   messageOverrides?: CompilerMessageOverrideMap;
 };

Diff for: developer/src/kmc-copy/src/KeymanProjectCopier.ts

@@ -4,7 +4,7 @@
  * Copy a keyboard or lexical model project
  */
 
-import { CloudUrls, GitHubUrls, CompilerCallbacks, CompilerLogLevel, KeymanCompiler, KeymanCompilerArtifact, KeymanCompilerArtifacts, KeymanCompilerResult, KeymanDeveloperProject, KeymanDeveloperProjectOptions, KPJFileReader, KPJFileWriter, KpsFileReader, KpsFileWriter, ValidIds } from "@keymanapp/developer-utils";
+import { CloudUrls, GitHubUrls, CompilerCallbacks, KeymanCompiler, KeymanCompilerArtifact, KeymanCompilerArtifacts, KeymanCompilerResult, KeymanDeveloperProject, KeymanDeveloperProjectOptions, KPJFileReader, KPJFileWriter, KpsFileReader, KpsFileWriter, ValidIds, CompilerBaseOptions } from "@keymanapp/developer-utils";
 import { KeymanFileTypes } from "@keymanapp/common-types";
 
 import { CopierMessages } from "./copier-messages.js";
@@ -19,12 +19,7 @@ type CopierFunction = (
  * @public
  * Options for the Keyman Developer project copier
  */
-export interface CopierOptions /* not inheriting from CompilerBaseOptions */ {
-  /**
-   * Reporting level to console, used by NodeCompilerCallbacks (not used in
-   * compiler modules; all messages are still reported to the internal log)
-   */
-  logLevel?: CompilerLogLevel;
+export interface CopierOptions extends CompilerBaseOptions {
   /**
    * output path where project folder will be created
    */

Diff for: developer/src/kmc-generate/src/abstract-generator.ts

@@ -4,7 +4,7 @@
  * Base interfaces and classes for generating Keyman Developer source files
  */
 
-import { CompilerCallbacks, CompilerLogLevel, KeymanCompilerArtifact, KeymanCompilerArtifacts, KeymanCompilerResult } from "@keymanapp/developer-utils";
+import { CompilerBaseOptions, CompilerCallbacks, KeymanCompilerArtifact, KeymanCompilerArtifacts, KeymanCompilerResult } from "@keymanapp/developer-utils";
 import { GeneratorMessages } from './generator-messages.js';
 import { KeymanTargets } from "@keymanapp/common-types";
 
@@ -13,12 +13,7 @@ import { KeymanTargets } from "@keymanapp/common-types";
  * @public
  * Options for the Keyman Developer project generator
  */
-export interface GeneratorOptions /* not inheriting from CompilerBaseOptions */ {
-  /**
-   * Reporting level to console, used by NodeCompilerCallbacks (not used in
-   * compiler modules; all messages are still reported to the internal log)
-   */
-  logLevel?: CompilerLogLevel;
+export interface GeneratorOptions extends CompilerBaseOptions {
   /**
    * identifier (basename) of the keyboard or model
    */

Diff for: developer/src/kmc-generate/src/basic-generator.ts

@@ -27,13 +27,13 @@ export class BasicGenerator extends AbstractGenerator {
   protected preGenerate() {
     const dt = new Date();
 
-    this.languageTags = this.options.languageTags.length
+    this.languageTags = this.options.languageTags?.length
       ? this.options.languageTags.map(tag => new Intl.Locale(tag).minimize().toString())
       : [this.DEFAULT_LOCALE];
 
     // Verify that targets passed in are valid
     let failed = false;
-    for(const target of this.options.targets) {
+    for(const target of this.options.targets ?? []) {
       if(!KeymanTargets.AllKeymanTargets.includes(target)) {
         this.callbacks.reportMessage(GeneratorMessages.Error_InvalidTarget({target}));
         failed = true;
@@ -43,7 +43,7 @@ export class BasicGenerator extends AbstractGenerator {
       return false;
     }
 
-    if(this.options.targets.includes(KeymanTargets.KeymanTarget.any) || this.options.targets.length == 0) {
+    if(this.options.targets?.includes(KeymanTargets.KeymanTarget.any) || !this.options.targets?.length) {
       this.resolvedTargets = KeymanTargets.AllKeymanTargets.filter(target => target != KeymanTargets.KeymanTarget.any);
     } else {
       this.resolvedTargets = [].concat(this.options.targets);

Diff for: developer/src/kmc/src/commands/analyze.ts

@@ -3,18 +3,14 @@ import * as path from 'path';
 import { Command, Option } from 'commander';
 import { NodeCompilerCallbacks } from '../util/NodeCompilerCallbacks.js';
 import { InfrastructureMessages } from '../messages/infrastructureMessages.js';
-import { CompilerCallbacks, CompilerLogLevel } from '@keymanapp/developer-utils';
+import { CompilerBaseOptions, CompilerCallbacks } from '@keymanapp/developer-utils';
 import { AnalyzeOskCharacterUse, AnalyzeOskRewritePua } from '@keymanapp/kmc-analyze';
 import { BaseOptions } from '../util/baseOptions.js';
 import { runOnFiles } from '../util/projectRunner.js';
 import { exitProcess } from '../util/sysexits.js';
+import { commanderOptionsToBaseOptions } from '../util/extendedCompilerOptions.js';
 
-interface AnalysisActivityOptions /* not inheriting from CompilerBaseOptions */ {
-  /**
-   * Reporting level to console, used by NodeCompilerCallbacks (not used in compiler modules;
-   * all messages are still reported to the internal log)
-   */
-  logLevel?: CompilerLogLevel;
+interface AnalysisActivityOptions extends CompilerBaseOptions {
   base?: string;
   stripDottedCircle?: boolean;
   includeCounts?: boolean;
@@ -30,7 +26,7 @@ export function declareAnalyze(program: Command) {
 
 function declareOskCharUse(command: Command) {
   let subCommand = command.command('osk-char-use [infile...]');
-  BaseOptions.addLogLevel(subCommand);
+  BaseOptions.addAll(subCommand);
   subCommand
     .description('Analyze On Screen Keyboards for character usage')
     .option('-b, --base', 'First PUA codepoint to use, in hexadecimal (default F100)', 'F100')
@@ -75,9 +71,24 @@ function declareOskRewrite(command: Command) {
     });
 }
 
+function commanderOptionsToAnalyzeOptions(options: any): AnalysisActivityOptions {
+  // TODO: split options per sub-command
+  const result: AnalysisActivityOptions = {
+    ...commanderOptionsToBaseOptions(options),
+    // AnalysisActivityOptions
+    base: options.base,
+    includeCounts: options.includeCounts ?? false,
+    inputMappingFile: options.inputMappingFile,
+    mappingFile: options.mappingFile,
+    stripDottedCircle: options.stripDottedCircle ?? false,
+  };
+  return result;
+}
+
 async function analyze(action: (callbacks: CompilerCallbacks, filenames: string[], options: AnalysisActivityOptions)=>Promise<boolean>,
-    filenames: string[], options: AnalysisActivityOptions): Promise<boolean> {
-  let callbacks = new NodeCompilerCallbacks({logLevel: options.logLevel});
+    filenames: string[], commanderOptions: any): Promise<boolean> {
+  const options = commanderOptionsToAnalyzeOptions(commanderOptions);
+  let callbacks = new NodeCompilerCallbacks(options);
   try {
     return await action(callbacks, filenames, options);
   } catch(e) {

Diff for: developer/src/kmc/src/commands/build.ts

@@ -7,20 +7,18 @@ import { NodeCompilerCallbacks } from '../util/NodeCompilerCallbacks.js';
 import { InfrastructureMessages } from '../messages/infrastructureMessages.js';
 import { KeymanFileTypes } from '@keymanapp/common-types';
 import { CompilerOptions, CompilerFileCallbacks } from '@keymanapp/developer-utils';
-import { BaseOptions } from '../util/baseOptions.js';
+import { BuildBaseOptions } from '../util/baseOptions.js';
 import { expandFileLists } from '../util/fileLists.js';
 import { isProject } from '../util/projectLoader.js';
 import { buildTestData } from './buildTestData/index.js';
 import { buildWindowsPackageInstaller } from './buildWindowsPackageInstaller/index.js';
-import { commandOptionsToCompilerOptions } from '../util/extendedCompilerOptions.js';
+import { commanderOptionsToCompilerOptions } from '../util/extendedCompilerOptions.js';
 import { exitProcess } from '../util/sysexits.js';
 
 export function declareBuild(program: Command) {
   // TODO: localization?
   const buildCommand = program
     .command('build')
-    .option('--color', 'Force colorization for log messages')
-    .option('--no-color', 'No colorization for log messages; if both omitted, detects from console')
 
     // These options are only used with build file but are included here so that
     // they are visible in `kmc build --help`
@@ -32,7 +30,7 @@ export function declareBuild(program: Command) {
     .option('--no-compiler-version', 'Exclude compiler version metadata from output')
     .option('--no-warn-deprecated-code', 'Turn off warnings for deprecated code styles');
 
-  BaseOptions.addAll(buildCommand);
+  BuildBaseOptions.addAll(buildCommand);
 
   buildCommand.command('file [infile...]', {isDefault: true})
     .description(`Compile one or more source files or projects ('file' subcommand is default).`)
@@ -74,7 +72,7 @@ function initialize(commanderOptions: any) {
   // We use a default callback instance when validating command line, but throw
   // it away once we have completed initialization
   const initializationCallbacks = new NodeCompilerCallbacks({});
-  const options = commandOptionsToCompilerOptions(commanderOptions, initializationCallbacks);
+  const options = commanderOptionsToCompilerOptions(commanderOptions, initializationCallbacks);
   return options;
 }
 

Diff for: developer/src/kmc/src/commands/buildTestData/index.ts

@@ -4,13 +4,13 @@ import * as kmcLdml from '@keymanapp/kmc-ldml';
 import { CompilerCallbacks, defaultCompilerOptions, LDMLKeyboardTestDataXMLSourceFile, LDMLKeyboardXMLSourceFileReader } from '@keymanapp/developer-utils';
 import { NodeCompilerCallbacks } from '../../util/NodeCompilerCallbacks.js';
 import { fileURLToPath } from 'url';
-import { CommandLineBaseOptions } from 'src/util/baseOptions.js';
+import { CommandLineBuildBaseOptions } from 'src/util/baseOptions.js';
 import { exitProcess } from '../../util/sysexits.js';
 import { InfrastructureMessages } from '../../messages/infrastructureMessages.js';
 import { dirname } from 'node:path';
 
 export async function buildTestData(infile: string, _options: any, commander: any): Promise<void> {
-  const options: CommandLineBaseOptions = commander.optsWithGlobals();
+  const options: CommandLineBuildBaseOptions = commander.optsWithGlobals();
 
   let compilerOptions: kmcLdml.LdmlCompilerOptions = {
     ...defaultCompilerOptions,

Diff for: developer/src/kmc/src/commands/buildWindowsPackageInstaller/index.ts

@@ -3,10 +3,10 @@ import * as path from 'path';
 import { CompilerCallbacks, defaultCompilerOptions } from '@keymanapp/developer-utils';
 import { NodeCompilerCallbacks } from '../../util/NodeCompilerCallbacks.js';
 import { WindowsPackageInstallerCompiler, WindowsPackageInstallerSources } from '@keymanapp/kmc-package';
-import { CommandLineBaseOptions } from 'src/util/baseOptions.js';
+import { CommandLineBuildBaseOptions } from 'src/util/baseOptions.js';
 import { exitProcess } from '../../util/sysexits.js';
 
-interface WindowsPackageInstallerCommandLineOptions extends CommandLineBaseOptions {
+interface WindowsPackageInstallerCommandLineOptions extends CommandLineBuildBaseOptions {
   msi: string;
   exe: string;
   license: string;

Diff for: developer/src/kmc/src/commands/copy.ts

@@ -10,12 +10,13 @@ import { InfrastructureMessages } from '../messages/infrastructureMessages.js';
 import { BaseOptions } from '../util/baseOptions.js';
 import { exitProcess } from '../util/sysexits.js';
 import { CopierOptions, KeymanProjectCopier } from '@keymanapp/kmc-copy';
+import { commanderOptionsToBaseOptions } from '../util/extendedCompilerOptions.js';
 
 /* c8 ignore start */
 
 export function declareCopy(program: Command) {
   const command = program.command('copy <project>');
-  BaseOptions.addLogLevel(command);
+  BaseOptions.addAll(command);
   command
     .description('Copy a Keyman keyboard or lexical model project')
     .option('-o, --out-path <path>', 'New name and path for project')
@@ -35,6 +36,8 @@ export function declareCopy(program: Command) {
 
 function commanderOptionsToCopierOptions(options: any): CopierOptions {
   const result: CopierOptions = {
+    ...commanderOptionsToBaseOptions(options),
+    // CopierOptions
     outPath: options.outPath,
     dryRun: options.dryRun ?? false,
     relocateExternalFiles: options.relocateExternal ?? false,
@@ -46,15 +49,16 @@ const MaxDryRunCopyLogMessages = 10000;
 
 async function copyProject(ids: string | string[], _options: any, commander: any): Promise<never|void> {
   const commanderOptions = commander.optsWithGlobals();
-  const callbacks = new NodeCompilerCallbacks({logLevel: commanderOptions.logLevel ?? 'info'});
-  if(!await doCopy(callbacks, ids, commanderOptions)) {
+  const options = commanderOptionsToCopierOptions(commanderOptions);
+  const callbacks = new NodeCompilerCallbacks(options);
+  if(!await doCopy(callbacks, ids, options)) {
     return await exitProcess(1);
   }
 }
 
 /* c8 ignore stop */
 
-async function doCopy(callbacks: NodeCompilerCallbacks, sources: string | string[], commanderOptions: any): Promise<boolean> {
+async function doCopy(callbacks: NodeCompilerCallbacks, sources: string | string[], options: CopierOptions): Promise<boolean> {
   const source = sources;
 
   if(!source || typeof source != 'string') {
@@ -64,8 +68,6 @@ async function doCopy(callbacks: NodeCompilerCallbacks, sources: string | string
     return false;
   }
 
-  const options = commanderOptionsToCopierOptions(commanderOptions);
-
   if(options.dryRun) {
     // For dry run, we may need a lot of log messages, to show
     // where all the files are copied to