Skip to content

cli: support --file option in config commands - #10005

Open
vaghinak-vardanyan wants to merge 2 commits into
jj-vcs:mainfrom
vaghinak-vardanyan:config-file-option
Open

cli: support --file option in config commands#10005
vaghinak-vardanyan wants to merge 2 commits into
jj-vcs:mainfrom
vaghinak-vardanyan:config-file-option

Conversation

@vaghinak-vardanyan

@vaghinak-vardanyan vaghinak-vardanyan commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Allow targeting a specific config file path with --file in
jj config {edit,set,unset}.

This enables explicit targeting of specific configuration files (such
as files in conf.d/ or custom files loaded via --config-file) and
avoids interactive prompts when multiple config files exist.

The --file option validates that the target path is a recognized jj
configuration location to prevent creating untracked or arbitrary files.

Additionally this changes the behavior of the jj config {edit,set,unset} --user
to target primary config file instead of an interactive window to choose between
that and conf.d/*.toml

Fixes #9541

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added/updated tests to cover my changes
  • I fully understand the code that I am submitting (what it does,
    how it works, how it's organized), including any code drafted by an LLM.
  • For any prose generated by an LLM, I have proof-read and copy-edited with
    an eye towards deleting anything that is irrelevant, clarifying anything
    that is confusing, and adding details that are relevant. This includes,
    for example, commit descriptions, PR descriptions, and code comments.

@vaghinak-vardanyan
vaghinak-vardanyan requested a review from a team as a code owner August 18, 2026 13:43
@vaghinak-vardanyan

Copy link
Copy Markdown
Contributor Author

cc @josephlou5 (as discussed on #9541) . As I don't have access yet to add reviewers.

Note: Added myself to docs/paid_contributors.md in #10006

Comment thread cli/src/commands/config/list.rs

@josephlou5 josephlou5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also update docs/config.md with some text about this flag?

Comment thread cli/src/commands/config/mod.rs Outdated
@joyously

Copy link
Copy Markdown

The "unattended automation" part seems dangerous, when other discussions were focused on the security of the config files.
Especially now that there have been AI models that have escaped their testing sandbox, is it smart to enable this sort of thing?

Comment thread cli/tests/cli-reference@.md.snap Outdated
@josephlou5

Copy link
Copy Markdown
Contributor

Does there also need to be any more handling for my comment #9541 (comment)? It would be nice if jj config edit/set/unset --user targeted the global <config_root>/.config/jj/config.toml file, and an explicit --file <config_root>/.config/jj/conf.d/blah.toml would target that specific file.

@vaghinak-vardanyan
vaghinak-vardanyan force-pushed the config-file-option branch 3 times, most recently from bed312f to d36171e Compare August 19, 2026 11:37
@vaghinak-vardanyan

Copy link
Copy Markdown
Contributor Author

Could you also update docs/config.md with some text about this flag?

Done.

@vaghinak-vardanyan

Copy link
Copy Markdown
Contributor Author

Does there also need to be any more handling for my comment #9541 (comment)? It would be nice if jj config edit/set/unset --user targeted the global <config_root>/.config/jj/config.toml file, and an explicit --file <config_root>/.config/jj/conf.d/blah.toml would target that specific file.

I agree that now as --file exists, it makes sense to restrict --user to the primary config file. Although I would like to change it in a separate commit/CL to basically keep the commits clean, like this one adds a new feature, another one changes a behavior. What do you think?

@vaghinak-vardanyan

Copy link
Copy Markdown
Contributor Author

The "unattended automation" part seems dangerous, when other discussions were focused on the security of the config files. Especially now that there have been AI models that have escaped their testing sandbox, is it smart to enable this sort of thing?

Just to clarify, the interactive prompt was only there to disambiguate between multiple config files, not as a security gate. Commands like jj config set --repo (and git config) were running non-interactively, and any process running jj could already edit the config files on disk directly anyway.

I also added path validation so --file can only target valid jj config locations. I've updated the wording in the changelog and commit message to avoid confusion!

@josephlou5 josephlou5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw you can resolve comment threads once you've addressed them: https://docs.jj-vcs.dev/latest/contributing/#code-reviews

Comment thread cli/src/commands/config/mod.rs Outdated
}
}

// 5. Check system config paths (/etc/jj/config.toml or files in /etc/jj/conf.d)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean jj config edit --file /etc/jj/config.toml will work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. As long as the user/process has write permissions for /etc/jj/ (e.g. running under sudo or as root), jj config {edit,set,unset} --file /etc/jj/config.toml will work and target it as ConfigSource::System.

If run without write permissions, it will fail with a standard permission denied error.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK this is new behavior. Should it be documented?

There's also a question of whether it should be possible, as I think these system files should usually be provided rather than available to be edited. Obviously if you have permission you can do whatever, but Jujutsu thus far has simply said "system configuration is loaded from here" and that was the end of the story. I guess there isn't a huge reason to not do this, but just saying it felt a little surprising to me :)

If we keep this, I think it should be documented.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the context! I don't feel strongly either way. My thinking was just consistency: since --file allows targeting specific config files, supporting system config paths felt natural. I've updated the docs accordingly.
I am also okay to remove if we eventually consider that. Let me know what you think.

Comment thread cli/src/commands/config/mod.rs Outdated
/// See `jj config edit` if you'd like to immediately edit a file.
#[derive(clap::Args, Clone, Debug)]
#[group(id = "config_level", multiple = false, required = true)]
pub struct ConfigPathArgs {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Maybe add a regular // comment explaining why the common ConfigTargetArgs can't be reused here? (Because this command doesn't accept --file.)

Comment thread cli/tests/test_config_command.rs
Comment thread cli/tests/test_config_command.rs
Comment thread cli/tests/test_config_command.rs
@josephlou5

Copy link
Copy Markdown
Contributor

Although I would like to change it in a separate commit/CL to basically keep the commits clean, like this one adds a new feature, another one changes a behavior. What do you think?

Yes, that sounds good. You can keep it in this PR if you'd like (and I would recommend that too).

Comment thread cli/src/commands/config/path.rs Outdated
/// Target the workspace-level config
#[arg(long)]
workspace: bool,
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you leave the original ConfigLevelArgs unmodified? I don't think jj config list should support --file=PATH either.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback. Yeah, I agree. I guess I misunderstood how list worked on the first pass. I've reverted ConfigLevelArgs, and introduced ConfigTargetArgs specifically for the modifying commands (edit, set, unset). Could you please have another look on that?

Comment thread cli/src/commands/config/mod.rs Outdated
@vaghinak-vardanyan
vaghinak-vardanyan force-pushed the config-file-option branch 2 times, most recently from a521699 to 8e8696f Compare August 24, 2026 00:20
Comment thread docs/config.md
Comment thread CHANGELOG.md Outdated
@vaghinak-vardanyan
vaghinak-vardanyan force-pushed the config-file-option branch 2 times, most recently from 9476eb6 to 7f8386a Compare August 24, 2026 13:30
Comment thread cli/src/commands/config/mod.rs Outdated
Comment thread cli/src/config.rs Outdated
Comment thread cli/src/config.rs Outdated
Comment thread cli/src/config.rs Outdated
Comment thread cli/src/config.rs Outdated
Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md Outdated
@vaghinak-vardanyan
vaghinak-vardanyan force-pushed the config-file-option branch 2 times, most recently from 069d007 to f5f836b Compare August 30, 2026 07:07
Comment thread cli/src/config.rs Outdated
Comment thread cli/src/config.rs Outdated
Comment thread cli/tests/test_config_command.rs Outdated
Comment thread cli/src/config.rs Outdated
Allow targeting a specific config file path with `--file` in
`jj config {edit,set,unset}`.

This enables explicit targeting of specific configuration files (such
as files in `conf.d/` or custom files loaded via `--config-file`) and
avoids interactive prompts when multiple config files exist.

The `--file` option validates that the target path is a recognized `jj`
configuration location to prevent creating untracked or arbitrary files.

Fixes jj-vcs#9541
@vaghinak-vardanyan
vaghinak-vardanyan force-pushed the config-file-option branch 2 times, most recently from 8b19193 to 1a93ab7 Compare August 31, 2026 06:57
When modifying user configuration (`jj config {edit,set,unset} --user`),
always target the primary user configuration file (`~/.config/jj/config.toml`
or `~/.jjconfig.toml`) rather than prompting when drop-in files exist in
`conf.d/`.

Specific drop-in files in `conf.d/` can now be targeted explicitly with
the `--file` option.

Fixes jj-vcs#9541
#[test]
fn test_config_set_file_with_existing_scopes() -> TestResult {
let mut test_env = TestEnvironment::default();
let conf_d = test_env.config_path().parent().unwrap().join("conf.d");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: test_env.env_root().join("conf.d") ?

Comment on lines +2226 to +2244
// Using JJ_CONFIG allows targeting any custom file with --file
let env_file = test_env.env_root().join("from_env.toml");
std::fs::write(&env_file, "")?;
let output = work_dir.run_jj_with(|cmd| {
cmd.env("JJ_CONFIG", &env_file);
cmd.args([
"config",
"set",
"--file",
env_file.to_str().unwrap(),
"env-key",
"env-val",
])
});
insta::assert_snapshot!(output, @"");
assert_eq!(
std::fs::read_to_string(&env_file)?,
"env-key = \"env-val\"\n"
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This seems redundant because JJ_CONFIG is set globally by the test runner.

Comment on lines +2207 to +2224
// Using global --config-file allows targeting any custom file with --file
let custom_file = test_env.env_root().join("outside.toml");
std::fs::write(&custom_file, "")?;
let output = work_dir.run_jj([
"--config-file",
custom_file.to_str().unwrap(),
"config",
"set",
"--file",
custom_file.to_str().unwrap(),
"outside-key",
"outside-val",
]);
insta::assert_snapshot!(output, @"");
assert_eq!(
std::fs::read_to_string(&custom_file)?,
"outside-key = \"outside-val\"\n"
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Do you think this behavior is useful? If not, can you update the comment to note that it can be removed if necessary?

Hint: Valid config locations include user configs (`~/.config/jj/config.toml` or `conf.d/*.toml`), repo/workspace configs, or files loaded with the global flag `--config-file <PATH>`.
[EOF]
[exit status: 1]
");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can you add a test for a valid relative path?

Comment on lines +858 to +913
fn test_config_set_user_with_conf_d() -> TestResult {
let mut test_env = TestEnvironment::default();
let config_dir = test_env.config_path().parent().unwrap();
let config_file = config_dir.join("config.toml");
let conf_d = config_dir.join("conf.d");
std::fs::create_dir_all(&conf_d)?;
std::fs::write(&config_file, "foo = 'from_config'\n")?;
std::fs::write(conf_d.join("work.toml"), "bar = 'from_work'\n")?;
let user_config_path = join_paths([&config_file, &conf_d])?;
test_env.set_config_path(&user_config_path);

// Setting with --user targets config.toml directly without prompting
let output = test_env.run_jj_in(".", ["config", "set", "--user", "foo", "updated_foo"]);
insta::assert_snapshot!(output, @"");
assert_eq!(
std::fs::read_to_string(&config_file)?,
"foo = \"updated_foo\"\n"
);
assert_eq!(
std::fs::read_to_string(conf_d.join("work.toml"))?,
"bar = 'from_work'\n"
);

Ok(())
}

#[test]
fn test_config_set_user_targets_first_file_when_only_conf_d_exists() -> TestResult {
let mut test_env = TestEnvironment::default();
let config_dir = test_env.config_path().parent().unwrap();
let config_file = config_dir.join("config.toml");
let conf_d = config_dir.join("conf.d");
std::fs::create_dir_all(&conf_d)?;
std::fs::write(conf_d.join("01_work.toml"), "bar = 'from_work'\n")?;
std::fs::write(conf_d.join("02_home.toml"), "baz = 'from_home'\n")?;
let user_config_path = join_paths([&config_file, &conf_d])?;
test_env.set_config_path(&user_config_path);

// config.toml does not exist
assert!(!config_file.exists());

// Setting with --user targets the first file in conf.d
let output = test_env.run_jj_in(".", ["config", "set", "--user", "foo", "new_foo"]);
insta::assert_snapshot!(output, @"");
assert!(!config_file.exists());
assert_eq!(
std::fs::read_to_string(conf_d.join("01_work.toml"))?,
"bar = 'from_work'\nfoo = \"new_foo\"\n"
);
assert_eq!(
std::fs::read_to_string(conf_d.join("02_home.toml"))?,
"baz = 'from_home'\n"
);

Ok(())
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests look redundant. We already have jj config set/edit with multiple files.

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.

FR: Allow unattended setting of user config when multiple config files exist

5 participants