microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.4k stars 8.3k forks source link

Settings editor saves settings.json lines with trailing whitespace #10887

Open mrjoel opened 3 years ago

mrjoel commented 3 years ago

Windows Terminal version (or Windows build number)

1.9.1942.0

Other Software

No response

Steps to reproduce

I have my settings.json tracked in a dotfiles git repo. Recently (now that settings UI is available) I've noticed that the settings.json is save with a trailing space after a name. This appears to be the case when the name is an array or object whose contents continue onto the next line (i.e. the space after the name is always added without considering whether a string value or array/object value will follow.

Expected Behavior

Lines are trimmed when saving

Actual Behavior

Extra spaces are left behind for entries which span multiple lines.

zadjii-msft commented 3 years ago

Can you give a specific example? I think I'm having a hard time trying to mentally picture what's wrong here.

mrjoel commented 3 years ago

Sure, the specific cases where I see this is happening on my settings.json are listed below, where an extra trailing whitespace character is added.

For all of these cases the space is added after the name and colon, but before the value is added, which happens to be on the next line. Instead, I'd expect either to trim each string, or better to use the context to only add the space separator if the next character to be added is not a newline. I haven't taken a look at the code to see how it's handled, but it's curious that .actions is not written with the trailing space, perhaps it gets special handling?

For the case of .actions[].command, what is actually emitted is:

{
    "$schema": "https://aka.ms/terminal-profiles-schema",
    "actions":
    [
        {
            "command": "find", //mrjoel: note that string values on single line are fine, but object values later are not
            "keys": "ctrl+shift+f"
        },
        {
            "command":␣ // mrjoel: '␣' visually represents literal space character here
            {
                "action": "splitPane",
                "split": "auto",
                "splitMode": "duplicate"
            },
            "keys": "alt+shift+d"
        }
    ],

What is expected is below, note the lack of trailing space (fake depicted as '␣' to be easier to visualize in the diff).

-            "command":␣
+            "command":
zadjii-msft commented 2 years ago

these docs for jsoncpp are relevant. IIRC the flag was enableYAMLCompatibility. I could have swore we had this, even some time after #2515

networkhermit commented 7 months ago

these docs for jsoncpp are relevant. IIRC the flag was enableYAMLCompatibility. I could have swore we had this, even some time after #2515

@zadjii-msft Hello. Does it suggest that it's an upstream issue? This issue still exists in Windows Terminal Version: 1.19.10573.0. Renaming settings.json and let the WT recreate a default json file have trailing whitespaces too.

zadjii-msft commented 6 months ago

I'm not sure! Someone would have to dig into the codebase and make sure we're setting that whenever we're saving the settings. If we are, then yea, I'd bet it's an upstream issue.

fallenwood commented 4 months ago

these docs for jsoncpp are relevant. IIRC the flag was enableYAMLCompatibility. I could have swore we had this, even some time after #2515

@zadjii-msft Hello. Does it suggest that it's an upstream issue? This issue still exists in Windows Terminal Version: 1.19.10573.0. Renaming settings.json and let the WT recreate a default json file have trailing whitespaces too.

Looks like a known upstream issue at jsoncpp https://github.com/open-source-parsers/jsoncpp/pull/1154