Skip to content
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

Allow to change directory from the next argument #1079

Closed
wants to merge 1 commit into from

Conversation

deeagle001
Copy link
Contributor

The -C change directory option only accepts dir within the same argv argument. The git tool allows this value to be in the next argument. This PR allows providing the change directory as the next argument after -C.

@koutcher
Copy link
Collaborator

koutcher commented Feb 8, 2021

To be honest, if I had to name one contribution I wished I didn't do it would probably be -C... It already required #911 to fix some regressions and I realize that I need -C (detect renames) more often than -C/path/to/repo. Actually, Git itself only accepts -C /path/to/repo and not -C/path/to/repo, so I would propose to go even further with:

diff --git a/src/tig.c b/src/tig.c
index ed4388f..f02566a 100644
--- a/src/tig.c
+++ b/src/tig.c
@@ -389,7 +389,7 @@ static const char usage_string[] =
 "  +<number>       Select line <number> in the first view\n"
 "  -v, --version   Show version and exit\n"
 "  -h, --help      Show help message and exit\n"
-"  -C<path>        Start in <path>";
+"  -C <path>       Start in <path>";
 
 void
 usage(const char *message)
@@ -489,9 +489,10 @@ parse_options(int argc, const char *argv[], bool pager_mode)
 	/* Options that must come before any subcommand. */
 	for (i = 1; i < argc; i++) {
 		const char *opt = argv[i];
-		if (!strncmp(opt, "-C", 2)) {
-			if (chdir(opt + 2))
-				die("Failed to change directory to %s", opt + 2);
+		if (!strcmp(opt, "-C") && i + 1 < argc && *argv[i + 1] != '-') {
+			const char *dir = argv[++i];
+			if (chdir(dir))
+				die("Failed to change directory to %s", dir);
 			continue;
 		} else {
 			break;

That would allow the -C /path/to/repo syntax and, in case -C is used alone, it would be passed to the second stage of command line parsing where it would be added to diff options. What do you think ?

@krobelus
Copy link
Contributor

krobelus commented Feb 9, 2021

Yeah, the earlier we change this, the better. With -C foo, shell compeltion will work OOTB.
Since Git doesn't allow -Cfoo, it feels okay to mimic that. It is breaking but avoids confusion in the long term.

koutcher pushed a commit that referenced this pull request Feb 10, 2021
This introduces an incompatibility with previous versions.

Replace the syntax `tig -C/path/to/repo` with `tig -C /path/to/repo` to be
consistent with Git and allow the use of `tig -C` to detect copies.

[tk: tweak patch and commit message]
@koutcher koutcher closed this Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants