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

FR: jj config unset [OPTIONS] <--user|--repo> <NAME> #4458

Open
Veykril opened this issue Sep 13, 2024 · 6 comments · May be fixed by #4547
Open

FR: jj config unset [OPTIONS] <--user|--repo> <NAME> #4458

Veykril opened this issue Sep 13, 2024 · 6 comments · May be fixed by #4547
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@Veykril
Copy link
Collaborator

Veykril commented Sep 13, 2024

Is your feature request related to a problem? Please describe.
Removing a configuration currently requires editing the file manually, having this command makes the removal of configs more convenient.

Describe the solution you'd like
Have a jj config subcommand that removes any set config by the given name.

Describe alternatives you've considered
None

Additional context
This would give parity with git's config command, which likewise offers an unset subcommand

@yuja yuja added the enhancement New feature or request label Sep 13, 2024
@PhilipMetzger PhilipMetzger added the good first issue Good for newcomers label Sep 14, 2024
@pylbrecht
Copy link

I have been lurking for a while now and this seems like a great opportunity to get involved.

Disclaimer: as a full-time working father of two my progress usually is slow but steady. In case that becomes a problem, let me know.

@pylbrecht pylbrecht linked a pull request Oct 1, 2024 that will close this issue
4 tasks
@pylbrecht
Copy link

pylbrecht commented Oct 11, 2024

Do we want to follow git's behavior for unset; removing empty tables, which are the result of an unset?

Example

Before

[table]
foo = true

[another-table]
bar = true

Unset another-table.bar, which results in another-table being empty:

$ jj config unset another-table.bar

After

[table]
foo = true

..and should such removals recursively bubble up to the config root?

Before

[table]
foo = true

[another-table]
inline-table = { foo = false }

Unset another-table.inline-table.foo, which results in another-table.inline-table being empty, which in turn results in another-table being empty:

$ jj unset another-table.inline-table.foo

After

[table]
foo = true

To me this is the most intuitive, but I would like to confirm before spending time on implementing it.

@arxanas
Copy link
Collaborator

arxanas commented Oct 11, 2024

There may be cases where an empty table is semantically meaningful. In particular, an inline empty table might indicate "enabled with default or no options". I've seen this pattern used productively in many domains.

Does Git have any cases where empty tables are semantically-meaningful? Actually – I guess in Git it's probably not possible to address a table by itself, but only its entries by pattern-matching on the key. It's more like they don't have tables in the actual data model, just keys with dots, and the table-like syntax is just shorthand, rather than representing a real nested object. That seems like a critical difference.

Likewise, Git represents lists with multivars, while TOML has real list objects. There's no empty list value in Git, while we can represent an empty list in jj. And it would probably be weird to automatically delete empty lists as the analogue to tables.

I don't know if jj has any cases of semantically-meaningful empty tables at present, but I could easily see it happening. I would advise to skip implementing the table deletion behavior for now and revisit later.

@pylbrecht
Copy link

There may be cases where an empty table is semantically meaningful. In particular, an inline empty table might indicate "enabled with default or no options". I've seen this pattern used productively in many domains.

Thanks a lot for your quick feedback! I was not aware of that.

Does Git have any cases where empty tables are semantically-meaningful? Actually – I guess in Git it's probably not possible to address a table by itself, but only its entries by pattern-matching on the key. It's more like they don't have tables in the actual data model, just keys with dots, and the table-like syntax is just shorthand, rather than representing a real nested object. That seems like a critical difference.

I was just performing a manual test with a simple example by adding the following to ~/.gitconfig:

[table]
foo = true

..and running:

$ git config --global --unset table.foo

..which resulted in table being deleted in .gitconfig.

I don't know if jj has any cases of semantically-meaningful empty tables at present, but I could easily see it happening. I would advise to skip implementing the table deletion behavior for now and revisit later.

I am more than happy to skip the table deletion, as I was already starting to bang my head against the wall a little while spiking the implementation.

@arxanas
Copy link
Collaborator

arxanas commented Oct 11, 2024

I was just performing a manual test

Not sure if I clearly expressed this: I think this is only an option for Git because the "tables" are not semantically meaningful as standalone entities, so Git can go ahead and delete the empty ones for convenience without issue. It makes sense for Git to do it for that reason [and then I explained why I believe that it doesn't make sense for TOML/us].

EDIT: I guess I didn't understand if you're providing evidence that Git does or doesn't have semantic tables when you wrote that, or if you were just providing more context/an example for the table deletion behavior in Git.

@pylbrecht
Copy link

EDIT: I guess I didn't understand if you're providing evidence that Git does or doesn't have semantic tables when you wrote that, or if you were just providing more context/an example for the table deletion behavior in Git.

The latter.

I was just trying to describe how I came to the idea of removing empty tables (poorly, as it seems to me 😅).
I had a hunch, though, that I can't just deduct expected behavior for jj based on "this is how Git does it for that very simple case", hoping for someone (like you for instance) to come along to provide a more informed view.
So.. thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants