microsoft / terminal

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

Saving action without ID immediately erases it; no warning/error #17557

Closed carlos-zamora closed 2 weeks ago

carlos-zamora commented 1 month ago

Windows Terminal version

1.22.1831.0

Windows build number

26250.5000

Other Software

N/A

Steps to reproduce

  1. open settings.json
  2. in actions add the following:
        {
            "command": {
                "action": "openSuggestions",
                "source": "all",
                "useCommandline": false
            }
        },

Expected Behavior

A warning should appear saying that the JSON is invalid and Terminal should either (1) ignore the changes or (2) fall back to default settings

Actual Behavior

settings.json is immediately rewritten and the text I just added in disappeared!

DHowett commented 1 month ago

I don't understand - it should have been assigned an ID, right?

carlos-zamora commented 1 month ago

Oh totally. The JSON snippet is intentionally invalid because it's missing an ID. But, in the past, invalid JSON settings throw a warning/error when they get loaded and we don't overwrite the settings.json. For some reason, in this case, we do, and the invalid JSON that gets added disappears.

Also, given that the actions/keybindings split is relatively new, I'm concerned that this is a common case users may hit. I just wanted to add openSuggestions to the command palette, so I added the command without a keybinding attached (common practice before the split). Instead, I immediately had the snippet deleted upon saving the file.

DHowett commented 1 month ago

Sorry, I mean: shouldn't the "expected behavior" be that we ADD an ID automatically?

carlos-zamora commented 1 month ago

ah, good call. yeah!

carlos-zamora commented 2 weeks ago

Looked into this further. Turns out, openSuggestions doesn't exist. The actual key is supposed to be showSuggestions.

Thus, closing.