jj should have a thing like git config ...

[jsoref]

Description

Learning to edit toml isn't going to be top of anyone's minds when they start using jj. A jj config would mean they don't need to

Steps to Reproduce the Problem

1. See jj should walk users through setting their email/name #530

Expected Behavior

jj config user.name '...'
jj config user.email '...'

Actual Behavior

% jj
Config error: invalid TOML value, did you mean to use a quoted string? at line 2 column 8 in ../../../.jjconfig.toml
% jj
Config error: expected newline, found a plus at line 3 column 16 in ../../../.jjconfig.toml

Specifications

• Platform:
• Version: 0.4.0

[dbarnett]

jj help config would also be a nice place to point to any reference docs about what options the config actually supports. See also #879 on reference docs.

[dbarnett]

I'd like to attempt this one. Seems like a good place to start to get familiar with the structure of the code before trying for some others I'm interested to tackle.

[martinvonz]

Great! Yes, this should be a very good starter task. You probably don't even need any pointers because it's probably just a matter of updating the command arguments (DescribeArgs) and implementation cmd_describe().

[dbarnett]

You're using describe as just an example, right? Or are you saying config relates to the describe command in particular?

[martinvonz]

Oops, I thought you (and I) were commenting on #328 :)

This one is a bit harder than #328, but probably still a good starter task. I hope there's some good crate out there for parsing TOML and writing it back, while preserving comments etc. Assuming there is, this is probably not too hard.

[dbarnett]

I hope there's some good crate out there for parsing TOML and writing it back, while preserving comments etc.

Yeah, I'll need to figure out all of that to support "set", but I just did "get" and "list" for now which are simpler.

[martinvonz]

This issue was about editing configs via jj config, so it should still be open

[dbarnett]

Oh yes, agreed. I think GitHub's detection was a little over-eager where I said "partially fixes".

[dbarnett]

Side comment: That GitHub spam from all the force-pushed commits is really annoying. Is that WAI or is there a better way I should be pushing commits?

[arxanas]

It's really annoying, so I personally try to keep the issue references in the pull request instead of the commit (which is unfortunate for other reasons)...

[dbarnett]

Planning to add a config set command like the FR originally requested and then we can call this Fixed and follow up on other improvements separately.

A few other follow-ups I'm planning to work on:

• Get config list supporting --user and --repo filters by taking advantage of LayeredConfigs (and improve the output ordering)
• Exclude the built-in defaults from the config list output (maybe with a --show-defaults option to manually enable them)

[dbarnett]

K, I'm forking these improvement ideas for config list into #1047. For this FR we'll focus on adding the remaining config set subcommand.

[dbarnett]

Any suggestion for the case where user config path is a directory? Some cases are straightforward, but when there are multiple files in the config dir precedence could get tricky.

Ideal could be:

• Empty dir => create a "config.toml" file
• Dir contains a single file => write to that file
• Dir contains multiple files => write config to highest precedence file

I'd like to not overthink this case, but unfortunately I can't really have a test for config set --user without doing something about that case (since the tests use a config directory with lots of files).

[jsoref]

Your proposal is reasonable...

[yuja]

I'd like to not overthink this case, but unfortunately I can't really have a test for config set --user without doing something about that case (since the tests use a config directory with lots of files).

I think it's also fine to error out if the config path is a directory.

In tests, we can override the path with .jj_cmd(..).env("JJ_CONFIG", ..).

[dbarnett]

Oh, true. Didn't think to override it to a file path.

One other gotcha will be inferring the value types. Easiest is to assume every value is a string, and maybe have an explicit arg for specifying other value types. Should also be possible to infer some types based on the config schema, but that's going to be a little complex to implement and will never be foolproof.

[martinvonz]

One other gotcha will be inferring the value types.

We have the global --config-toml, so should we have --set-toml? I think we should use toml syntax anyway. Btw, I'm very open to dropping the -toml part of --config-toml and say that it's implied. Should jj config --set foo.bar baz be an error because baz is not a valid toml value? Or do we interpret invalid toml as string?

[dbarnett]

I think requiring toml syntax will be awkward. Just to set a simple string value you'd need two layers of quoting like set user.email '"[email protected]"'. 99% of the usefulness would be setting a string, number, or bool value. For setting arrays, tables, etc you'd probably want to use edit instead of set anyway. We could mostly infer that if it doesn't look like a number or boolean it's a string value, except if you did want a literal string value like "1.1" or "true" it gets a little weird.

Probably time to go check how mercurial and git handle these cases...

[dbarnett]

Probably time to go check how mercurial and git handle these cases...

Oh, mercurial doesn't have any direct set command AFAICT, just commands to get and edit. git has a --type arg, but also looks like it doesn't need to be as type-aware since the cfg format isn't so explicit about quoting etc.

Considering the whole command is a convenience helper, I'd vote to keep it pretty simplistic and infer number/boolean/string with a --type=number|bool|string option to force the type.

[yuja]

I'd vote to keep it pretty simplistic and infer number/boolean/string with a --type=number|bool|string option to force the type.

config::Config isn't strict about underlying value types, so I think it's okay to take everything as a string. If we add a proper support for config schema, value type can be determined by config key.

[dbarnett]

Okay! It's certainly not perfect, but we have config subcommands list/set/edit and you can configure email/name as requested in the FR, like: jj config set --user user.email "[email protected]".

We might still want to reference that new command in the hint for new users to set up name/email at https://github.com/martinvonz/jj/blob/99cb0ba7c5cef0d8b53dbfe64a168764e50a4dc2/src/cli_util.rs#L1045, though.