-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Added support for inline scripting #15
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { assertEquals, assertThrows } from "../../../dev_deps.ts"; | ||
import replaceEnvVars from "../replaceEnvVars.ts"; | ||
|
||
const CMD = "deno run ${SOME_ENV_VAR_1} ${SOME_ENV_VAR_2} mod.js"; | ||
const EMPTY_ENV = {}; | ||
const ENV = { | ||
SOME_ENV_VAR_1: "VALUE_1", | ||
SOME_ENV_VAR_2: "VALUE_2", | ||
}; | ||
const REPLACED_CMD = "deno run VALUE_1 VALUE_2 mod.js"; | ||
|
||
Deno.test("should throw no cmd error", async () => { | ||
assertThrows((): void => { | ||
replaceEnvVars("", EMPTY_ENV); | ||
}, ReferenceError); | ||
}); | ||
|
||
Deno.test("should return the cmd that entered as is", async () => { | ||
const cmd = replaceEnvVars(CMD, EMPTY_ENV); | ||
|
||
assertEquals(cmd, CMD); | ||
}); | ||
|
||
Deno.test("should return the replaced cmd", async () => { | ||
const cmd = replaceEnvVars(CMD, ENV); | ||
|
||
assertEquals(cmd, REPLACED_CMD); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
// TODO - Update this with the dotenv interface. | ||
import { DotenvConfig } from "../interfaces.ts"; | ||
|
||
/** | ||
* This is added here in anticipation of https://github.com/BentoumiTech/denox/issues/12 being completed. | ||
* There are many time we need to inject env variables into a script. | ||
* These comments are merely to give you an insight to my thinking and would be removed after a successful review. | ||
*/ | ||
|
||
function replaceEnvVars(cmd: string | undefined, env: DotenvConfig): string { | ||
const envKeys: string[] = Object.keys(env); | ||
|
||
if (!cmd) { | ||
throw new ReferenceError("A command is required by this function."); | ||
} | ||
|
||
// I'm not sure if you have a loop preference? This is the method I have become accustom to in terms of readability and performance. | ||
envKeys.forEach((envKey: string) => { | ||
// This is a little ugly but I like explicit variable declaration for readability. | ||
const envValue: string = env[envKey]; | ||
// String are immutable so it doesn't matter if we directly work on the cmd string. | ||
cmd = cmd?.replace(`\${${envKey}}`, envValue); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about changing the format from I prefer the second as it's the way POSIX commands utilize environment variables, it's the same way you would write in a bash terminal for example. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am completely fine with changing that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI, in POSIX shells While |
||
}); | ||
|
||
return cmd; | ||
} | ||
|
||
export default replaceEnvVars; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
forEach
looks good for me