GCodeProcessor: fix ratos postprocess with --overwrite-input but …

…without `--idex`. (#43)

* GCodeProcessor: fix ratos postprocess with --overwrite-input but without --idex.

* Util: add object-manipulation.ts, notably including strictWithDefaults.

* GCodeProcessor: use strictWithDefaults for options sanitization.

src/cli/commands/postprocessor.tsx

@@ -337,14 +337,17 @@ export const postprocessor = (program: Command) => {
 				// My current opinion: rather overprocess than underprocess. Specific situations where skipping processing is OK
 				// can be handled in fancy ways later - i need to see that it's worth the complexity first.
 
+				// NOTE: processGCode is blind as to whether the input file requires transformation to be ready to print. Notably,
+				// in the --overwrite-input and not --idex scenario, the processGCode call will not actually lead to transformation,
+				// but only analysis. The fullAnalysis option applies only is this non-transformative scenario.
 				const result =
 					outputFile != null && outputFile.trim() !== ''
-						? await processGCode(inputFile, outputFile, opts)
-						: await inspectGCode(inputFile, { ...opts, fullInspection: false });
+						? await processGCode(inputFile, outputFile, { ...opts, fullAnalysis: false })
+						: await inspectGCode(inputFile, { ...opts, fullAnalysis: false });
 
 				getLogger().info(result, 'postprocessor result');
 
-				if (!result.wasAlreadyProcessed && args.overwriteInput) {
+				if (!result.wasAlreadyProcessed && args.overwriteInput && result.isProcessed) {
 					getLogger().info({ outputFile, inputFile }, 'renaming output file to input file');
 					fs.renameSync(outputFile, inputFile);
 				}

src/server/gcode-processor/GCodeFile.ts

@@ -39,6 +39,7 @@ import { AssertionError } from 'node:assert';
 import { AnalysisResult, AnalysisResultKind, AnalysisResultSchema } from '@/server/gcode-processor/AnalysisResult';
 import { Printability } from '@/server/gcode-processor/Printability';
 import { NullSink } from '@/server/gcode-processor/NullSink';
+import { PartialToNullableRequired, strictWithDefaults } from '@/utils/object-manipulation';
 
 function assert(condition: any, message?: string): asserts condition {
 	if (!condition) {
@@ -70,12 +71,37 @@ export type TransformOptions = { progressTransform?: Transform } & Pick<
 	GCodeProcessorOptions,
 	'abortSignal' | 'allowUnsupportedSlicerVersions' | 'onWarning' | 'printerHasIdex'
 >;
+
 export type AnalyseOptions = { progressTransform?: Transform } & GCodeProcessorOptions;
+
 export type InspectOptions = Pick<
 	GCodeProcessorOptions,
 	'onWarning' | 'allowUnsupportedSlicerVersions' | 'printerHasIdex'
 >;
 
+const defaultTransformOptions: PartialToNullableRequired<TransformOptions> = {
+	abortSignal: null,
+	progressTransform: null,
+	allowUnsupportedSlicerVersions: null,
+	onWarning: null,
+	printerHasIdex: null,
+};
+
+const defaultAnalyseOptions: PartialToNullableRequired<AnalyseOptions> = {
+	abortSignal: null,
+	progressTransform: null,
+	allowUnsupportedSlicerVersions: null,
+	onWarning: null,
+	printerHasIdex: null,
+	quickInspectionOnly: null,
+};
+
+const defaultInspectOptions: PartialToNullableRequired<InspectOptions> = {
+	allowUnsupportedSlicerVersions: null,
+	onWarning: null,
+	printerHasIdex: null,
+};
+
 const fsReaderGetLines = util.promisify(fsReader) as (path: string, lines: number) => Promise<string>;
 
 /** Match a block like:
@@ -133,6 +159,8 @@ export class GCodeFile {
 
 	/** Factory. Returns GCodeFile with valid `info` or throws if the file header can't be parsed etc. */
 	public static async inspect(path: string, options: InspectOptions): Promise<GCodeFile> {
+		// Sanitise options to remove any extra properties that might be present at runtime.
+		options = strictWithDefaults(options, defaultInspectOptions);
 		const onWarning = options?.onWarning;
 		const header = await fsReaderGetLines(path, 4);
 		const gci = GCodeFile.tryParseHeader(header);
@@ -277,6 +305,8 @@ export class GCodeFile {
 
 	/** If the current file is already processed by the current GCodeHandling version, throws; otherwise, inputFile will be deprocessed on the fly (if already processed) and (re)transformed. */
 	public async transform(outputFile: string, options: TransformOptions): Promise<GCodeInfo> {
+		// Sanitise options to remove any extra properties that might be present at runtime.
+		options = strictWithDefaults(options, defaultTransformOptions);
 		let fh: FileHandle | undefined;
 		const gcodeProcessor = new GCodeProcessor(options);
 		const encoder = new BookmarkingBufferEncoder(undefined, undefined, options.abortSignal);
@@ -341,6 +371,8 @@ export class GCodeFile {
 
 	/** If the current file is already processed by the current GCodeHandling version, returns inputFile.info; otherwise, inputFile will be unprocessed on the fly (if already processed) and (re)analysed. */
 	public async analyse(options: AnalyseOptions): Promise<GCodeInfo> {
+		// Sanitise options to remove any extra properties that might be present at runtime.
+		options = strictWithDefaults(options, defaultAnalyseOptions);
 		const gcodeProcessor = new GCodeProcessor(options);
 
 		try {

src/server/gcode-processor/gcode-processor.ts

@@ -52,20 +52,22 @@ interface CommonOptions {
 	 * will return a result with printability 'UNKNOWN'.
 	 */
 	allowUnknownGenerator?: boolean;
+	/**
+	 * If true, the whole file is examined, and a full {@link AnalysisResult} is built. Otherwise,
+	 * a quick analysis is performed, and at most the `gcodeInfo`, `firstMoveX` and `firstMoveY`
+	 * fields will be populated.
+	 *
+	 * Note: this option has no effect when transformation is performed. It only applies when
+	 * non-transformative analysis is performed.
+	 */
+	fullAnalysis?: boolean;
 }
 
 interface ProcessOptions extends CommonOptions {
 	overwrite?: boolean;
 }
 
-interface InspectOptions extends CommonOptions {
-	/**
-	 * If true, the whole file is examined, and a full {@link AnalysisResult} is built. Otherwise,
-	 * a quick inspection is performed, and at most the `gcodeInfo`, `firstMoveX` and `firstMoveY`
-	 * fields will be populated.
-	 */
-	fullInspection?: boolean;
-}
+interface InspectOptions extends CommonOptions {}
 
 export async function inspectGCode(inputFile: string, options: InspectOptions): Promise<ProcessorResult> {
 	const inputStat = await stat(path.resolve(inputFile));
@@ -78,7 +80,7 @@ export async function inspectGCode(inputFile: string, options: InspectOptions):
 		printerHasIdex: options.idex,
 		allowUnsupportedSlicerVersions: options.allowUnsupportedSlicerVersions,
 		allowUnknownGenerator: options.allowUnknownGenerator,
-		quickInspectionOnly: !options.fullInspection,
+		quickInspectionOnly: !options.fullAnalysis,
 		abortSignal: options.abortSignal,
 		onWarning: options.onWarning,
 	};
@@ -144,6 +146,7 @@ export async function processGCode(
 		printerHasIdex: options.idex,
 		allowUnsupportedSlicerVersions: options.allowUnsupportedSlicerVersions,
 		allowUnknownGenerator: options.allowUnknownGenerator,
+		quickInspectionOnly: !options.fullAnalysis,
 		abortSignal: options.abortSignal,
 		onWarning: options.onWarning,
 	};
@@ -180,7 +183,22 @@ export async function processGCode(
 		}
 	}
 
-	if (gcf.printability !== Printability.MUST_PROCESS) {
+	if (gcf.printability === Printability.READY && !gcf.info.analysisResult) {
+		let progressStream: Transform | undefined;
+
+		if (options.onProgress) {
+			progressStream = progress({ length: inputStat.size });
+			progressStream.on('progress', options.onProgress);
+		}
+
+		return {
+			...(await gcf.analyse({ progressTransform: progressStream, ...gcfOptions })).serialize(),
+			wasAlreadyProcessed: false,
+			printability: gcf.printability,
+			printabilityReasons: gcf.printabilityReasons,
+			canDeprocess: gcf.canDeprocess,
+		};
+	} else if (gcf.printability !== Printability.MUST_PROCESS) {
 		return {
 			...gcf.info.serialize(),
 			wasAlreadyProcessed: gcf.info.isProcessed,

src/utils/object-manipulation.ts

@@ -0,0 +1,43 @@
+export type NullableRequiredToPartial<T> = Partial<{ [key in keyof T]: NonNullable<T[key]> }>;
+
+export const removeNulledProperties = <T extends object>(opts: T): NullableRequiredToPartial<T> => {
+	const result = { ...opts };
+	Object.keys(opts).forEach((opt) => {
+		if (result[opt as keyof T] === null) {
+			delete result[opt as keyof T];
+		}
+	});
+	return result;
+};
+
+export type PartialToNullableRequired<T> = Required<{ [key in keyof T]: T[key] | null }>;
+
+/**
+ * Typically used with `options` objects where most or all propeties are optional, returns a copy of {@link opts} which
+ * contains only the properties of {@link T}. Properties of {@link T} that are not defined by {@link opts} will be replaced
+ * by the default values in {@link defaults}. By design, {@link defaults} must declare all the properties of {@link T},
+ * using value `null` if there is no default value for a given property. Properties that are not defined by either
+ * {@link opts} or {@link defaults} will be absent from the returned object.
+ *
+ * Example:
+ * @example
+ * const defaultOptions: PartialToNullableRequired<TransformOptions> = {
+ *    abortSignal: null,
+ *    progressTransform: null,
+ *    allowUnsupportedSlicerVersions: false,
+ *    onWarning: () => {},
+ *    printerHasIdex: false,
+ *    quickInspectionOnly: false,
+ * };
+ * strictWithDefaults(test, defaultOptions);
+ * */
+export const strictWithDefaults = <T extends object, S extends PartialToNullableRequired<T>>(
+	opts: Partial<T>,
+	structure: [PartialToNullableRequired<T>] extends [S] ? S : `STRUCTURE_DOES_NOT_MATCH_INPUT`,
+): NullableRequiredToPartial<T> => {
+	return removeNulledProperties(
+		Object.fromEntries(
+			Object.keys(structure).map((k) => [k, opts[k as keyof T] ?? null]),
+		) as NullableRequiredToPartial<T>,
+	);
+};