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

jj config list does not escape keys enough to produce valid TOML #1322

Closed
ilyagr opened this issue Mar 1, 2023 · 0 comments · Fixed by #3725
Closed

jj config list does not escape keys enough to produce valid TOML #1322

ilyagr opened this issue Mar 1, 2023 · 0 comments · Fixed by #3725

Comments

@ilyagr
Copy link
Collaborator

ilyagr commented Mar 1, 2023

For example, jj config list --include-defaults includes such lines as:

colors.op_log current_operation user="yellow"
template-aliases.format_short_id(id)="id.shortest(6)"

In both cases, the key would need to be quoted to be valid TOML. It is theoretically possible we'd even have a key that includes quotes or backslashes some day, which would need escaping.

This might be considered a part of #1047.

@ilyagr ilyagr changed the title jj config list does not escape keys properly jj config list does not escape keys to make valid TOML Mar 1, 2023
@ilyagr ilyagr changed the title jj config list does not escape keys to make valid TOML jj config list does not escape keys enough to produce valid TOML Mar 1, 2023
ilyagr added a commit to ilyagr/jj that referenced this issue May 21, 2024
Fixes martinvonz#1322. There may be more places where keys need escaping, I'm not
completely sure.
ilyagr added a commit to ilyagr/jj that referenced this issue May 21, 2024
Fixes martinvonz#1322. There may be more places where keys need escaping, I'm not
completely sure.
ilyagr added a commit that referenced this issue May 21, 2024
Fixes #1322. There may be more places where keys need escaping, I'm not
completely sure.
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 a pull request may close this issue.

1 participant