zellij-org / zellij

A terminal workspace with batteries included
https://zellij.dev
MIT License
22.1k stars 673 forks source link

Difference in error handling of .kdl files from 0.36.0 to 0.37.2 - improve theme file error handling #2598

Open ESPR3SS0 opened 1 year ago

ESPR3SS0 commented 1 year ago

Basic information

zellij --version: 0.37.2 uname -av or ver(Windows): Linux MSI 5.15.90.1-microsoft-standard-WSL2

Further information I updated zellij from version 0.36.0 to 0.37.2 and did not change any configuration files. Upon running zellij it immediately quit and print De serialization error: Expected a valid node entry. to the terminal. Nothing was added to the log file.

After reinstalling zellij version 0.36.0 it ran successfully, while still logging the following:

ERROR  |zellij_utils::setup      | 2023-07-01 14:41:24.688 [main      ]
 [/home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zellij-utils-0.36.0/src/setup.rs:325]: error loading theme file: 
KdlDeserializationError(KdlError { input: "themes {\n  catppuccin-latte{\n    bg: \"#acb0be\" # Surface2\n    fg: \"#acb0be\" # Surface2\n  
  red: \"#d20f39\"\n    green: \"#40a02b\"\n    blue: \"#1e66f5\"\n    yellow: \"#df8e1d\"\n    magenta: \"#ea76cb\" # Pink\n    orange: 
\"#fe640b\" # Peach\n    cyan: \"#04a5e5\" # Sky\n    black: \"#4c4f69\" # Text\n    white: \"#dce0e8\" # Crust\n    }\n  catppuccin-frappe 
{\n    bg: \"#626880\" # Surface2\n    fg: \"#c6d0f5\"\n    red: \"#e78284\"\n    green: \"#a6d189\"\n    blue: \"#8caaee\"\n    yellow: 
\"#e5c890\"\n    magenta: \"#f4b8e4\" # Pink\n    orange: \"#ef9f76\" # Peach\n    cyan: \"#99d1db\" # Sky\n    black: \"#292c3c\" # 
Mantle\n    white: \"#c6d0f5\"\n    }\n\n  catppuccin-macchiato {\n    bg: \"#5b6078\" # Surface2\n    fg: \"#cad3f5\"\n    red: 
\"#ed8796\"\n    green: \"#a6da95\"\n    blue: \"#8aadf4\"\n    yellow: \"#eed49f\"\n    magenta: \"#f5bde6\" # Pink\n    orange: 
\"#f5a97f\" # Peach\n    cyan: \"#91d7e3\" # Sky\n    black: \"#1e2030\" # Mantle\n    white: \"#cad3f5\"\n    }\n\n  catppuccin-mocha {\n
    bg: \"#585b70\" # Surface2\n    fg: \"#cdd6f4\"\n    red: \"#f38ba8\"\n    green: \"#a6e3a1\"\n    blue: \"#89b4fa\"\n    yellow: 
\"#f9e2af\"\n    magenta: \"#f5c2e7\" # Pink\n    orange: \"#fab387\" # Peach\n    cyan: \"#89dceb\" # Sky\n    black: \"#181825\" # 
Mantle\n    white: \"#cdd6f4\"\n    }\n}\n", span: SourceSpan { offset: SourceOffset(47), length: 1 }, label: Some("plain identifiers can't be
 used here"), help: Some("If this was supposed to be a string, wrap it in quotes.\nIf this was supposed to be a new node, terminate the 
previous node with `;` or a newline."), kind: Context("a valid node entry") }) 

There was a syntax error in a theme definition file. This theme file was in the theme directory and did not have a theme that was currently being used.

The difference of handling the error between the 2 version is what I consider the bug:

I would believe the intended behavior in Version 0.37.2 would be the same behavior as Version 0.36.2 .

This was not an easy error to diagnose as there was little information given. I had no change in configuration and not much of a direction to head towards in order to solve it. Zellij so nicely handles config.kdl syntax errors, and such an implementation for handling theme configuration files would be helpful for users.

Incorrectly formatted file for reference

themes {
  catppuccin-latte{
    bg: "#acb0be" # Surface2
    fg: "#acb0be" # Surface2
    red: "#d20f39"
    green: "#40a02b"
    blue: "#1e66f5"
    yellow: "#df8e1d"
    magenta: "#ea76cb" # Pink
    orange: "#fe640b" # Peach
    cyan: "#04a5e5" # Sky
    black: "#4c4f69" # Text
    white: "#dce0e8" # Crust
    }
  catppuccin-frappe {
    bg: "#626880" # Surface2
    fg: "#c6d0f5"
    red: "#e78284"
    green: "#a6d189"
    blue: "#8caaee"
    yellow: "#e5c890"
    magenta: "#f4b8e4" # Pink
    orange: "#ef9f76" # Peach
    cyan: "#99d1db" # Sky
    black: "#292c3c" # Mantle
    white: "#c6d0f5"
    }

  catppuccin-macchiato {
    bg: "#5b6078" # Surface2
    fg: "#cad3f5"
    red: "#ed8796"
    green: "#a6da95"
    blue: "#8aadf4"
    yellow: "#eed49f"
    magenta: "#f5bde6" # Pink
    orange: "#f5a97f" # Peach
    cyan: "#91d7e3" # Sky
    black: "#1e2030" # Mantle
    white: "#cad3f5"
    }

  catppuccin-mocha {
    bg: "#585b70" # Surface2
    fg: "#cdd6f4"
    red: "#f38ba8"
    green: "#a6e3a1"
    blue: "#89b4fa"
    yellow: "#f9e2af"
    magenta: "#f5c2e7" # Pink
    orange: "#fab387" # Peach
    cyan: "#89dceb" # Sky
    black: "#181825" # Mantle
    white: "#cdd6f4"
    }
}
imsnif commented 1 year ago

Apologies for the trouble - I agree this is a regression.