BREAKING CHANGE: completely overhaul arg parsing & allow invoking CLI w/o initialBranch if cached

...as long as the initialBranch was already provided
in some previous invocation of stacked-rebase, thus cached.
sometime later, we could have a config for the default initial branch..

- create generic utils for argv parsing (separate package soon?)
- introduce multiple tests, which caught lots of (mostly new) bugs!
- tidy up code at start of gitStackedRebase, which now makes great sense & looks nice
  - same in other places
- clear up mental model of "parsing options" - it's "resolve" instead of "parse"

- move initialBranch parsing into resolveOptions
  - don't want out of sync `resolvedOptions.initialBranch` vs `nameOfInitialBranch`

- get rid of "intial branch is optional for some args" -- non issue
  - because e.g. for --continue to have any meaning, you'd have to have already
    started a stacked rebase & thus would have the initial branch cached.

Author: kiprasmel

README.md

@@ -86,15 +86,15 @@ git-stacked-rebase <branch>
     2. but will not apply the changes to partial branches until --apply is used.
 
 
-git-stacked-rebase <branch> [-a|--apply]
+git-stacked-rebase [-a|--apply]
 
-    1. will apply the changes to partial branches,
-    2. but will not push any partial branches to a remote until --push is used.
+    3. will apply the changes to partial branches,
+    4. but will not push any partial branches to a remote until --push is used.
 
 
-git-stacked-rebase <branch> [-p|--push -f|--force]
+git-stacked-rebase [-p|--push -f|--force]
 
-    1. will push partial branches with --force (and extra safety).
+    5. will push partial branches with --force (and extra safety).
 
 
 

argparse/argparse.spec.ts

@@ -0,0 +1,69 @@
+#!/usr/bin/env ts-node-dev
+
+import assert from "assert";
+
+import { NonPositional, NonPositionalWithValue, eatNonPositionals, eatNonPositionalsWithValues } from "./argparse";
+
+export function argparse_TC() {
+	eatNonPositionals_singleArg();
+	eatNonPositionals_multipleArgs();
+
+	eatNonPositionalsWithValues_singleArg();
+	eatNonPositionalsWithValues_multipleArgs();
+}
+
+function eatNonPositionals_singleArg() {
+	const argv = ["origin/master", "--autosquash", "foo", "bar"];
+	const targetArgs = ["--autosquash", "--no-autosquash"];
+	const expected: NonPositional[] = [{ argName: "--autosquash", origIdx: 1 }];
+	const expectedLeftoverArgv = ["origin/master", "foo", "bar"];
+
+	const parsed = eatNonPositionals(targetArgs, argv);
+
+	assert.deepStrictEqual(parsed, expected);
+	assert.deepStrictEqual(argv, expectedLeftoverArgv);
+}
+function eatNonPositionals_multipleArgs() {
+	const argv = ["origin/master", "--autosquash", "foo", "bar", "--no-autosquash", "baz"];
+	const targetArgs = ["--autosquash", "--no-autosquash"];
+	const expected: NonPositional[] = [
+		{ argName: "--autosquash", origIdx: 1 },
+		{ argName: "--no-autosquash", origIdx: 4 },
+	];
+	const expectedLeftoverArgv = ["origin/master", "foo", "bar", "baz"];
+
+	const parsed = eatNonPositionals(targetArgs, argv);
+
+	assert.deepStrictEqual(parsed, expected);
+	assert.deepStrictEqual(argv, expectedLeftoverArgv);
+}
+
+function eatNonPositionalsWithValues_singleArg() {
+	const argv = ["origin/master", "--git-dir", "~/.dotfiles", "foo", "bar"];
+	const targetArgs = ["--git-dir", "--gd"];
+	const expected: NonPositionalWithValue[] = [{ argName: "--git-dir", origIdx: 1, argVal: "~/.dotfiles" }];
+	const expectedLeftoverArgv = ["origin/master", "foo", "bar"];
+
+	const parsed = eatNonPositionalsWithValues(targetArgs, argv);
+
+	assert.deepStrictEqual(parsed, expected);
+	assert.deepStrictEqual(argv, expectedLeftoverArgv);
+}
+function eatNonPositionalsWithValues_multipleArgs() {
+	const argv = ["origin/master", "--git-dir", "~/.dotfiles", "foo", "bar", "--misc", "miscVal", "unrelatedVal"];
+	const targetArgs = ["--git-dir", "--gd", "--misc"];
+	const expected: NonPositionalWithValue[] = [
+		{ argName: "--git-dir", origIdx: 1, argVal: "~/.dotfiles" },
+		{ argName: "--misc", origIdx: 5, argVal: "miscVal" },
+	];
+	const expectedLeftoverArgv = ["origin/master", "foo", "bar", "unrelatedVal"];
+
+	const parsed = eatNonPositionalsWithValues(targetArgs, argv);
+
+	assert.deepStrictEqual(parsed, expected);
+	assert.deepStrictEqual(argv, expectedLeftoverArgv);
+}
+
+if (!module.parent) {
+	argparse_TC();
+}

argparse/argparse.ts

@@ -0,0 +1,140 @@
+import assert from "assert";
+
+export type Maybe<T> = T | undefined;
+
+export type Argv = string[];
+export type MaybeArg = Maybe<string>;
+
+/**
+ * parses the argv.
+ * mutates the `argv` array.
+ */
+export function createArgParse(argv: Argv) {
+	const getArgv = (): Argv => argv;
+	const peakNextArg = (): MaybeArg => argv[0];
+	const eatNextArg = (): MaybeArg => argv.shift();
+	const hasMoreArgs = (): boolean => argv.length > 0;
+
+	return {
+		getArgv,
+		peakNextArg,
+		eatNextArg,
+		hasMoreArgs,
+		eatNonPositionals: (argNames: string[]) => eatNonPositionals(argNames, argv),
+		eatNonPositionalsWithValues: (argNames: string[]) => eatNonPositionalsWithValues(argNames, argv),
+	};
+}
+
+export type NonPositional = {
+	origIdx: number;
+	argName: string;
+};
+
+export type NonPositionalWithValue = NonPositional & {
+	argVal: string;
+};
+
+export function eatNonPositionals(
+	argNames: string[],
+	argv: Argv,
+	{
+		howManyItemsToTakeWhenArgMatches = 1, //
+	} = {}
+): NonPositional[] {
+	const argMatches = (idx: number) => argNames.includes(argv[idx]);
+	let matchedArgIndexes: NonPositional["origIdx"][] = [];
+
+	for (let i = 0; i < argv.length; i++) {
+		if (argMatches(i)) {
+			for (let j = 0; j < howManyItemsToTakeWhenArgMatches; j++) {
+				matchedArgIndexes.push(i + j);
+			}
+		}
+	}
+
+	if (!matchedArgIndexes.length) {
+		return [];
+	}
+
+	const nonPositionalsWithValues: NonPositional[] = [];
+	for (const idx of matchedArgIndexes) {
+		nonPositionalsWithValues.push({
+			origIdx: idx,
+			argName: argv[idx],
+		});
+	}
+
+	const shouldRemoveArg = (idx: number) => matchedArgIndexes.includes(idx);
+	const argvIndexesToRemove: number[] = [];
+
+	for (let i = 0; i < argv.length; i++) {
+		if (shouldRemoveArg(i)) {
+			argvIndexesToRemove.push(i);
+		}
+	}
+
+	removeArrayValuesAtIndices(argv, argvIndexesToRemove);
+
+	return nonPositionalsWithValues;
+}
+
+export function eatNonPositionalsWithValues(argNames: string[], argv: Argv): NonPositionalWithValue[] {
+	const argsWithTheirValueAsNextItem: NonPositional[] = eatNonPositionals(argNames, argv, {
+		howManyItemsToTakeWhenArgMatches: 2,
+	});
+
+	assert.deepStrictEqual(argsWithTheirValueAsNextItem.length % 2, 0, `expected all arguments to have a value.`);
+
+	const properArgsWithValues: NonPositionalWithValue[] = [];
+	for (let i = 0; i < argsWithTheirValueAsNextItem.length; i += 2) {
+		const arg = argsWithTheirValueAsNextItem[i];
+		const val = argsWithTheirValueAsNextItem[i + 1];
+
+		properArgsWithValues.push({
+			origIdx: arg.origIdx,
+			argName: arg.argName,
+			argVal: val.argName,
+		});
+	}
+
+	return properArgsWithValues;
+}
+
+/**
+ * internal utils
+ */
+
+export function removeArrayValuesAtIndices<T>(arrayRef: T[], indexesToRemove: number[]): void {
+	/**
+	 * go in reverse.
+	 *
+	 * because if went from 0 to length,
+	 * removing an item from the array would adjust all other indices,
+	 * which creates a mess & needs extra handling.
+	 */
+	const indexesBigToSmall = [...indexesToRemove].sort((A, B) => B - A);
+
+	for (const idxToRemove of indexesBigToSmall) {
+		arrayRef.splice(idxToRemove, 1);
+	}
+
+	return;
+}
+
+/**
+ * common utilities for dealing w/ parsed values:
+ */
+
+export function maybe<T, S, N>(
+	x: T, //
+	Some: (x: T) => S,
+	None: (x?: never) => N
+) {
+	if (x instanceof Array) {
+		return x.length ? Some(x) : None();
+	}
+
+	return x !== undefined ? Some(x) : None();
+}
+
+export const last = <T>(xs: T[]): T => xs[xs.length - 1];

config.ts

@@ -2,7 +2,7 @@
 
 import Git from "nodegit";
 
-import { SomeOptionsForGitStackedRebase } from "./options";
+import { SpecifiableGitStackedRebaseOptions } from "./options";
 import { getGitConfig__internal } from "./internal";
 
 export const configKeyPrefix = "stackedrebase" as const;
@@ -15,25 +15,52 @@ export const configKeys = {
 
 export async function loadGitConfig(
 	repo: Git.Repository,
-	specifiedOptions: SomeOptionsForGitStackedRebase
+	specifiedOptions: SpecifiableGitStackedRebaseOptions
 ): Promise<Git.Config> {
 	return getGitConfig__internal in specifiedOptions
 		? await specifiedOptions[getGitConfig__internal]!({ GitConfig: Git.Config, repo })
 		: await Git.Config.openDefault();
 }
 
 export type ConfigValues = {
-	gpgSign: boolean;
-	autoApplyIfNeeded: boolean;
-	autoSquash: boolean;
+	gpgSign: boolean | undefined;
+	autoApplyIfNeeded: boolean | undefined;
+	autoSquash: boolean | undefined;
 };
 
-export async function parseGitConfigValues(config: Git.Config): Promise<ConfigValues> {
+export async function resolveGitConfigValues(config: Git.Config): Promise<ConfigValues> {
+	const [
+		gpgSign, //
+		autoApplyIfNeeded,
+		autoSquash,
+	] = await Promise.all([
+		resolveConfigBooleanValue(config.getBool(configKeys.gpgSign)),
+		resolveConfigBooleanValue(config.getBool(configKeys.autoApplyIfNeeded)),
+		resolveConfigBooleanValue(config.getBool(configKeys.autoSquash)),
+	]);
+
 	const configValues: ConfigValues = {
-		gpgSign: !!(await config.getBool(configKeys.gpgSign).catch(() => 0)),
-		autoApplyIfNeeded: !!(await config.getBool(configKeys.autoApplyIfNeeded).catch(() => 0)),
-		autoSquash: !!(await config.getBool(configKeys.autoSquash).catch(() => 0)),
+		gpgSign,
+		autoApplyIfNeeded,
+		autoSquash,
 	};
 
 	return configValues;
 }
+
+/**
+ * there's a difference between a value set to false (intentionally disabled),
+ * vs not set at all:
+ * can then look thru lower level options providers, and take their value.
+ *
+ * ```
+ * export const handleConfigBooleanValue = (x: Promise<number>) => x.then(Boolean).catch(() => undefined);
+ * ```
+ *
+ * but actually, it doesn't matter here, because when we're trying to resolve (here),
+ * our goal is to provide a final value that will be used by the program,
+ * thus no `undefined`.
+ *
+ */
+//
+export const resolveConfigBooleanValue = (x: Promise<number>) => x.then(Boolean).catch(() => false);

filenames.ts

@@ -10,6 +10,8 @@ export const filenames = {
 	gitRebaseTodo: "git-rebase-todo",
 	//
 	postStackedRebaseHook: "post-stacked-rebase",
+	//
+	initialBranch: "initial-branch",
 
 	/**
 	 * TODO extract others into here