Skip to content

Commit

Permalink
change trace and info options to live under logging object, upd…
Browse files Browse the repository at this point in the history
…ate tests, tweak logging
  • Loading branch information
suchipi committed Dec 1, 2023
1 parent cfad20a commit cba082b
Show file tree
Hide file tree
Showing 23 changed files with 492 additions and 284 deletions.
3 changes: 2 additions & 1 deletion src/api/commands/cd/cd-and-pwd.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ test("cd affects working directory of exec", async () => {
{
"code": 0,
"error": false,
"stderr": "",
"stderr": "exec: sh -c "echo $PWD"
",
"stdout": "<rootDir>/src
",
}
Expand Down
4 changes: 3 additions & 1 deletion src/api/commands/which/which.help.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ declare function which(
options?: {
searchPaths?: Array<Path | string>;
suffixes?: Array<string>;
trace?: (...args: Array<any>) => void;
logging?: {
trace?: (...args: Array<any>) => void;
};
}
): Path | null;
```
30 changes: 29 additions & 1 deletion src/api/commands/which/which.inc.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,36 @@
declare function which(
binaryName: string,
options?: {
/**
* A list of folders where programs may be found. Defaults to
* `env.PATH?.split(Path.OS_ENV_VAR_SEPARATOR) || []`.
*/
searchPaths?: Array<Path | string>;

/**
* A list of filename extension suffixes to include in the search, ie
* `[".exe"]`. Defaults to {@link Path.OS_PROGRAM_EXTENSIONS}.
*/
suffixes?: Array<string>;
trace?: (...args: Array<any>) => void;

/** Options which control logging. */
logging?: {
/**
* If provided, this logging function will be called multiple times as
* `which` runs, to help you understand what's going on and/or troubleshoot
* things. In most cases, it makes sense to use a function from `console`
* here, like so:
*
* ```js
* which("bash", {
* logging: { trace: console.log }
* });
* ```
*
* Defaults to the current value of {@link logger.trace}. `logger.trace`
* defaults to a no-op function.
*/
trace?: (...args: Array<any>) => void;
};
}
): Path | null;
8 changes: 6 additions & 2 deletions src/api/commands/which/which.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,14 @@ test("which", async () => {
which("sample", {
suffixes: [".one", ".two"],
trace: console.error,
logging: {
trace: console.error,
},
}),
which("program2", {
trace: console.error,
logging: {
trace: console.error,
},
}),
]);
`;
Expand Down
25 changes: 19 additions & 6 deletions src/api/commands/which/which.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ import { setHelpText } from "../../help";
import whichHelpText from "./which.help.md";
import { assert } from "../../assert";
import { types } from "../../types";
import { is } from "../../is";
import { quote } from "../../strings";
import { logger } from "../../logger";

function optionDefaults() {
return {
searchPaths: env.PATH?.split(Path.OS_ENV_VAR_SEPARATOR) || [],
suffixes: Array.from(Path.OS_PROGRAM_EXTENSIONS),
trace: logger.trace,
logging: {
trace: logger.trace,
},
};
}

Expand All @@ -21,7 +24,9 @@ export function which(
options?: {
searchPaths?: Array<Path | string>;
suffixes?: Array<string>;
trace?: (...args: Array<any>) => void;
logging?: {
trace?: (...args: Array<any>) => void;
};
}
): Path | null {
assert.type(binaryName, types.string, "'binaryName' must be a string");
Expand All @@ -46,17 +51,25 @@ export function which(
"when present, 'options.suffixes' must be an Array of strings"
);
assert.type(
options.trace,
types.or(types.undefined, types.Function),
"when present, 'options.trace' must be a Function"
options.logging,
types.or(types.undefined, types.object),
"when present, 'options.logging' must be an object"
);

if (is(options.logging, types.object)) {
assert.type(
options.logging.trace,
types.or(types.undefined, types.Function),
"when present, 'options.logging.trace' must be a Function"
);
}
}

const defaults = optionDefaults();
const {
searchPaths = defaults.searchPaths,
suffixes = defaults.suffixes,
trace = defaults.trace,
logging: { trace = defaults.logging.trace } = defaults.logging,
} = options ?? defaults;

for (const lookupPath of searchPaths) {
Expand Down
3 changes: 2 additions & 1 deletion src/api/env/env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ test("setting env affects child processes", async () => {
{
"code": 0,
"error": false,
"stderr": "",
"stderr": "exec: <yavascript binary> -e "env.BLAH_BLAH"
",
"stdout": "yes
",
}
Expand Down
7 changes: 4 additions & 3 deletions src/api/exec/ChildProcess.help.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Generally, you should not need to use the `ChildProcess` class directly, and sho
```ts
// Defined in yavascript/src/api/exec/ChildProcess.ts
declare class ChildProcess {
new(
constructor(
args: string | Path | Array<string | number | Path>,
options?: {
cwd?: string | Path;
Expand All @@ -17,7 +17,9 @@ declare class ChildProcess {
out?: FILE;
err?: FILE;
};
trace?: (...args: Array<any>) => void;
logging?: {
trace?: (...args: Array<any>) => void;
};
}
): ChildProcess;

Expand All @@ -30,7 +32,6 @@ declare class ChildProcess {
out: FILE;
err: FILE;
};
trace?: (...args: Array<any>) => void;

pid: number | null;
start(): number;
Expand Down
22 changes: 11 additions & 11 deletions src/api/exec/ChildProcess.inc.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ declare interface ChildProcess {
err: FILE;
};

/**
* Optional trace function which, if present, will be called at various times
* to provide information about the lifecycle of the process.
*/
trace?: (...args: Array<any>) => void;

pid: number | null;

/** Spawns the process and returns its pid (process id). */
Expand Down Expand Up @@ -68,11 +62,17 @@ declare type ChildProcessOptions = {
err?: FILE;
};

/**
* Optional trace function which, if present, will be called at various times
* to provide information about the lifecycle of the process.
*/
trace?: (...args: Array<any>) => void;
/** Options which control logging */
logging?: {
/**
* Optional trace function which, if present, will be called at various
* times to provide information about the lifecycle of the process.
*
* Defaults to the current value of {@link logger.trace}. `logger.trace`
* defaults to a function which writes to stderr.
*/
trace?: (...args: Array<any>) => void;
};
};

declare interface ChildProcessConstructor {
Expand Down
34 changes: 26 additions & 8 deletions src/api/exec/ChildProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ export type ChildProcessOptions = {
out?: FILE;
err?: FILE;
};
trace?: (...args: Array<any>) => void;
logging?: {
trace?: (...args: Array<any>) => void;
};
};

export class ChildProcess {
Expand All @@ -32,7 +34,10 @@ export class ChildProcess {
out: FILE;
err: FILE;
};
trace: (...args: Array<any>) => void;

private _logging: {
trace: (...args: Array<any>) => void;
};

pid: number | null = null;

Expand Down Expand Up @@ -95,18 +100,20 @@ export class ChildProcess {
"when present, 'stdio.err' option must be a FILE object"
);

this.trace = options.trace ?? logger.trace;
this._logging = {
trace: options.logging?.trace ?? logger.trace,
};

assert.type(
this.trace,
this._logging.trace,
types.Function,
"when present, 'options.trace' must be a function"
"when present, 'options.logging.trace' must be a function"
);
}

/** returns pid */
start(): number {
this.trace.call(null, "ChildProcess.start:", this.args);
this._logging.trace.call(null, "ChildProcess.start:", this.args);

this.pid = os.exec(this.args, {
block: false,
Expand Down Expand Up @@ -134,11 +141,22 @@ export class ChildProcess {
if (ret == pid) {
if (os.WIFEXITED(status)) {
const ret = { status: os.WEXITSTATUS(status), signal: undefined };
this.trace.call(null, "ChildProcess result:", this.args, "->", ret);
this._logging.trace.call(
null,
"ChildProcess result:",
this.args,
"->",
ret
);
return ret;
} else if (os.WIFSIGNALED(status)) {
const ret = { status: undefined, signal: os.WTERMSIG(status) };
this.trace.call(null, "ChildProcess result:", this.args, "->");
this._logging.trace.call(
null,
"ChildProcess result:",
this.args,
"->"
);
return ret;
}
}
Expand Down
25 changes: 13 additions & 12 deletions src/api/exec/exec.help.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,13 @@ The intent is that it behaves similarly to what you would expect from a UNIX she

`exec` also supports a second argument, an options object which supports the following keys (all are optional):

| Property | Purpose |
| ------------------------------ | ----------------------------------------------------- |
| cwd (string) | current working directory for the child process |
| env (object) | environment variables for the process |
| failOnNonZeroStatus (boolean) | whether to throw error on nonzero exit status |
| captureOutput (boolean/string) | controls how stdout/stderr is directed |
| trace (function) | used to log detailed info about the process execution |
| info (function) | used to log info about the process execution |
| Property | Purpose |
| ------------------------------ | ----------------------------------------------- |
| cwd (string) | current working directory for the child process |
| env (object) | environment variables for the process |
| failOnNonZeroStatus (boolean) | whether to throw error on nonzero exit status |
| captureOutput (boolean/string) | controls how stdout/stderr is directed |
| logging (object) | controls how/whether info messages are logged |

The return value of `exec` varies depending on the options passed:

Expand All @@ -55,11 +54,13 @@ declare function exec(
/** Defaults to `env` */
env?: { [key: string | number]: string | number | boolean };

/** Defaults to `undefined` */
trace?: (...args: Array<any>) => void;
logging?: {
/** Defaults to `logger.trace`. */
trace?: (...args: Array<any>) => void;

/** Defaults to {@link logger.info}. */
info?: (...args: Array<any>) => void;
/** Defaults to `logger.info`. */
info?: (...args: Array<any>) => void;
}

/** Defaults to true */
failOnNonZeroStatus?: boolean;
Expand Down
46 changes: 28 additions & 18 deletions src/api/exec/exec.inc.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,34 @@ declare type BaseExecOptions = {
/** Sets environment variables within the process. */
env?: { [key: string | number]: string | number | boolean };

/**
* If provided, this function will be called multiple times as `exec`
* runs, to help you understand what's going on and/or troubleshoot things.
* In most cases, it makes sense to use a logging function here, like so:
*
* ```js
* exec(["echo", "hi"], { trace: console.log });
* ```
*/
trace?: (...args: Array<any>) => void;

/**
* An optional, user-provided logging function to be used for informational
* messages.
*
* Defaults to {@link logger.info}.
*/
info?: (...args: Array<any>) => void;
/** Options which control logging. */
logging?: {
/**
* If provided, this logging function will be called multiple times as
* `exec` runs, to help you understand what's going on and/or troubleshoot
* things. In most cases, it makes sense to use a function from `console`
* here, like so:
*
* ```js
* exec(["echo", "hi"], {
* logging: { trace: console.log },
* });
* ```
*
* Defaults to the current value of {@link logger.trace}. `logger.trace`
* defaults to a no-op function.
*/
trace?: (...args: Array<any>) => void;

/**
* An optional, user-provided logging function to be used for informational
* messages. Less verbose than `logging.trace`.
*
* Defaults to the current value of {@link logger.info}. `logger.info`
* defaults to a function which logs to stderr.
*/
info?: (...args: Array<any>) => void;
};

/**
* Whether an Error should be thrown when the process exits with a nonzero
Expand Down
Loading

0 comments on commit cba082b

Please sign in to comment.