Skip to content

Commit

Permalink
BREAKING CHANGE: completely overhaul arg parsing & allow invoking CLI…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
kiprasmel committed Apr 23, 2023
1 parent f10fe31 commit f3d9bff
Show file tree
Hide file tree
Showing 19 changed files with 714 additions and 260 deletions.
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).



Expand Down
69 changes: 69 additions & 0 deletions argparse/argparse.spec.ts
Original file line number Diff line number Diff line change
@@ -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();
}
140 changes: 140 additions & 0 deletions argparse/argparse.ts
Original file line number Diff line number Diff line change
@@ -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];
45 changes: 36 additions & 9 deletions config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
2 changes: 2 additions & 0 deletions filenames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ export const filenames = {
gitRebaseTodo: "git-rebase-todo",
//
postStackedRebaseHook: "post-stacked-rebase",
//
initialBranch: "initial-branch",

/**
* TODO extract others into here
Expand Down
Loading

0 comments on commit f3d9bff

Please sign in to comment.