Skip to content

Commit 733064b

Browse files
committed
refactor: get rid of --view-todo-only
we never used it, code around it was messy, & probably had undiscovered bugs too. Signed-off-by: Kipras Melnikovas <kipras@kipras.org>
1 parent 0dfc945 commit 733064b

File tree

2 files changed

+6
-95
lines changed

2 files changed

+6
-95
lines changed

git-stacked-rebase.ts

Lines changed: 4 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -70,49 +70,8 @@ export async function gitStackedRebase(
7070
const pathToRegularRebaseDirInsideDotGit: string = path.join(dotGitDirPath, "rebase-merge");
7171
const pathToRegularRebaseTodoFile = path.join(pathToRegularRebaseDirInsideDotGit, filenames.gitRebaseTodo);
7272

73-
const createPathForStackedRebase = (withName: string): string => path.join(dotGitDirPath, withName); // "stacked-rebase"
74-
75-
const __default__pathToStackedRebaseDirInsideDotGit: string = createPathForStackedRebase("stacked-rebase");
76-
const __default__pathToStackedRebaseTodoFile = path.join(
77-
__default__pathToStackedRebaseDirInsideDotGit,
78-
filenames.gitRebaseTodo
79-
);
80-
81-
let parsed: {
82-
pathToStackedRebaseDirInsideDotGit: string;
83-
pathToStackedRebaseTodoFile: string;
84-
};
85-
86-
if (options.viewTodoOnly) {
87-
/**
88-
* would've been in stacked-rebase/
89-
* now will be in stacked-rebase/tmp/
90-
*/
91-
const insideDir: string = createPathForStackedRebase("stacked-rebase.tmp");
92-
93-
parsed = {
94-
pathToStackedRebaseDirInsideDotGit: insideDir,
95-
pathToStackedRebaseTodoFile: path.join(insideDir, filenames.gitRebaseTodo),
96-
};
97-
} else {
98-
parsed = {
99-
pathToStackedRebaseDirInsideDotGit: __default__pathToStackedRebaseDirInsideDotGit,
100-
pathToStackedRebaseTodoFile: __default__pathToStackedRebaseTodoFile,
101-
};
102-
}
103-
104-
/**
105-
* TODO v1: FIXME update usage of this to use the `__default__` instead (in most places except --view-todo)
106-
* EDIT: oh but this is what's actually proctecting from e.g. --apply together w/ --view-todo...
107-
*
108-
* TODO v2.1: instead, extract the `--view-todo` logic & exit early
109-
* TODO v2.2: though, also consider we'll want `--dry-run` in the future & the current approach might be better.
110-
*
111-
* TODO v3: okay nevermind re: v1, and yes v2.2 -- now that we're completely isolating into a separate dir
112-
* if it's --view-todo,
113-
*/
114-
const pathToStackedRebaseDirInsideDotGit: string = parsed.pathToStackedRebaseDirInsideDotGit;
115-
const pathToStackedRebaseTodoFile: string = parsed.pathToStackedRebaseTodoFile;
73+
const pathToStackedRebaseDirInsideDotGit: string = path.join(dotGitDirPath, "stacked-rebase");
74+
const pathToStackedRebaseTodoFile: string = path.join(pathToStackedRebaseDirInsideDotGit, filenames.gitRebaseTodo);
11675

11776
const checkIsRegularRebaseStillInProgress = (): boolean => fs.existsSync(pathToRegularRebaseDirInsideDotGit);
11877

@@ -272,7 +231,7 @@ export async function gitStackedRebase(
272231
// })
273232
);
274233

275-
if (!wasRegularRebaseInProgress || options.viewTodoOnly) {
234+
if (!wasRegularRebaseInProgress) {
276235
try {
277236
const editor: EitherEditor =
278237
editor__internal in options
@@ -300,16 +259,6 @@ export async function gitStackedRebase(
300259
}
301260
}
302261

303-
if (options.viewTodoOnly) {
304-
fs.rmdirSync(pathToStackedRebaseDirInsideDotGit, { recursive: true });
305-
306-
const dirname = path.basename(pathToStackedRebaseDirInsideDotGit);
307-
308-
process.stdout.write(`removed ${dirname}/\n`);
309-
310-
return;
311-
}
312-
313262
const regularRebaseTodoLines: string[] = [];
314263

315264
/**
@@ -1462,15 +1411,6 @@ git-stacked-rebase <branch> [--push|-p --force|-f]
14621411
2.1 unless autoApply is enabled.
14631412
14641413
1465-
git-stacked-rebase <branch> [-v|--view-todo|--view-only]
1466-
1467-
1. will make git-stacked-rebase work inside a separate, .tmp directory,
1468-
to allow viewing/editing (w/o affecting the actual todo
1469-
nor any subsequent runs that might happen later),
1470-
2. will NOT execute the rebase,
1471-
3. after viewing/editing, will remove the .tmp directory.
1472-
1473-
14741414
14751415
non-positional args:
14761416
@@ -1579,29 +1519,12 @@ git-stacked-rebase ${gitStackedRebaseVersionStr} __BUILD_DATE_REPLACEMENT_STR__
15791519
*/
15801520
const second = peakNextArg();
15811521

1582-
/**
1583-
* `isViewTodoOnly` is safe because the decision of using the .tmp directory or not
1584-
* is decided purely by this option,
1585-
* and **it's impossible to have more options** that would
1586-
* have side effects for the git repository
1587-
* (because git-stacked-rebase would be working in the same .tmp directory).
1588-
*
1589-
* i.e. if --view-todo is specified, then another option,
1590-
* such as --edit-todo, or --apply, cannot be specified,
1591-
* because all of these options are positional
1592-
* & are the 3rd argument.
1593-
* additionally, gitStackedRebase checks for these incompatible options
1594-
* in the library code as well (TODO check all incompatible options).
1595-
*
1596-
*/
1597-
const isViewTodoOnly: boolean =
1598-
!!second && ["--view-todo", "-v", "--view-only", "--view-todo-only"].includes(second);
15991522
const isApply: boolean = !!second && ["--apply", "-a"].includes(second);
16001523
const isContinue: boolean = !!second && ["--continue", "-c"].includes(second);
16011524
const isPush: boolean = !!second && ["--push", "-p"].includes(second);
16021525
const isBranchSequencer: boolean = !!second && ["--branch-sequencer", "--bs", "-s"].includes(second);
16031526

1604-
if (isViewTodoOnly || isContinue || isApply || isPush || isBranchSequencer) {
1527+
if (isContinue || isApply || isPush || isBranchSequencer) {
16051528
eatNextArg();
16061529
}
16071530

@@ -1650,7 +1573,6 @@ git-stacked-rebase ${gitStackedRebaseVersionStr} __BUILD_DATE_REPLACEMENT_STR__
16501573
const options: SomeOptionsForGitStackedRebase = {
16511574
gitDir,
16521575
autoSquash: isAutoSquash,
1653-
viewTodoOnly: isViewTodoOnly,
16541576
apply: isApply,
16551577
continue: isContinue,
16561578
push: isPush,

options.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { removeUndefinedProperties } from "./util/removeUndefinedProperties";
88

99
import { InternalOnlyOptions } from "./internal";
1010
import { parseGitConfigValues } from "./config";
11+
import { noop } from "./util/noop";
1112

1213
export type BaseOptionsForGitStackedRebase = {
1314
gitDir: string;
@@ -27,7 +28,6 @@ export type BaseOptionsForGitStackedRebase = {
2728
autoSquash?: boolean | undefined;
2829
autoApplyIfNeeded?: boolean | undefined;
2930

30-
viewTodoOnly: boolean;
3131
apply: boolean;
3232
continue: boolean;
3333
push: boolean;
@@ -104,24 +104,13 @@ export const getDefaultOptions = (): OptionsForGitStackedRebase => ({
104104
//
105105
branchSequencer: false,
106106
branchSequencerExec: false,
107-
//
108-
viewTodoOnly: false,
109107
});
110108

111109
export function areOptionsIncompatible(
112110
options: ParsedOptionsForGitStackedRebase, //
113111
reasons: string[] = []
114112
): boolean {
115-
if (options.viewTodoOnly) {
116-
if (options.apply) reasons.push("--apply cannot be used together with --view-todo");
117-
if (options.continue) reasons.push("--continue cannot be used together with --view-todo");
118-
if (options.push) reasons.push("--push cannot be used together with --view-todo");
119-
if (options.forcePush) reasons.push("--push --force cannot be used together with --view-todo");
120-
if (options.branchSequencer) reasons.push("--branch-sequencer cannot be used together with --view-todo");
121-
if (options.branchSequencerExec)
122-
reasons.push("--branch-sequencer --exec cannot be used together with --view-todo");
123-
}
124-
113+
noop(options);
125114
/**
126115
* TODO HANDLE ALL CASES
127116
*/

0 commit comments

Comments
 (0)