Repository: zellij-org/zellij (language: rust)

Issue:
Environment variables inside cwd
Hi, and thank you for this amazing project!

I'm trying to define a layout which I'll share with my teammates, and it includes paths that contain env variables that may be different between users. For example, I want to do something like:
```
layout {
  pane name="profiling" cwd="/somewhere/$USER/profiling"
}
```
where `$USER` is such environment variable.
When I open zellij with such layout, it silently falls back to current dir.

I actually consider the current behavior a bug. IMO it should either fail with an error, or do the right thing. (I realize that "the right thing" here is complicated because of `cwd` composition.)
Adding a section on "what happens if a path is invalid", and "what paths are valid" to the creating-a-layout doc page will also be helpful.

Thanks again!


Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 1c580f3280..68db479959 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -814,6 +814,15 @@ dependencies = [
  "dirs-sys",
 ]
 
+[[package]]
+name = "dirs"
+version = "4.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ca3aa72a6f96ea37bbc5aa912f6788242832f75369bdfdadcb0e38423f100059"
+dependencies = [
+ "dirs-sys",
+]
+
 [[package]]
 name = "dirs-sys"
 version = "0.3.7"
@@ -2529,6 +2538,15 @@ dependencies = [
  "opaque-debug 0.3.0",
 ]
 
+[[package]]
+name = "shellexpand"
+version = "3.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "dd1c7ddea665294d484c39fd0c0d2b7e35bbfe10035c5fe1854741a57f6880e1"
+dependencies = [
+ "dirs 4.0.0",
+]
+
 [[package]]
 name = "signal-hook"
 version = "0.1.17"
@@ -2880,7 +2898,7 @@ version = "0.7.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "76971977e6121664ec1b960d1313aacfa75642adc93b9d4d53b247bd4cb1747e"
 dependencies = [
- "dirs",
+ "dirs 2.0.2",
  "fnv",
  "nom 5.1.2",
  "phf 0.8.0",
@@ -4056,6 +4074,7 @@ dependencies = [
  "rmp-serde",
  "serde",
  "serde_json",
+ "shellexpand",
  "signal-hook 0.3.14",
  "strip-ansi-escapes",
  "strum",
diff --git a/zellij-utils/Cargo.toml b/zellij-utils/Cargo.toml
index 76ca72c90c..6c29f1e435 100644
--- a/zellij-utils/Cargo.toml
+++ b/zellij-utils/Cargo.toml
@@ -38,6 +38,7 @@ miette = { version = "3.3.0", features = ["fancy"] }
 regex = "1.5.5"
 tempfile = "3.2.0"
 kdl = { version = "4.5.0", features = ["span"] }
+shellexpand = "3.0.0"
 
 #[cfg(not(target_family = "wasm"))]
 [target.'cfg(not(target_family = "wasm"))'.dependencies]
diff --git a/zellij-utils/src/kdl/kdl_layout_parser.rs b/zellij-utils/src/kdl/kdl_layout_parser.rs
index 9af0561de7..21441cf173 100644
--- a/zellij-utils/src/kdl/kdl_layout_parser.rs
+++ b/zellij-utils/src/kdl/kdl_layout_parser.rs
@@ -333,22 +333,27 @@ impl<'a> KdlLayoutParser<'a> {
             (None, None) => None,
         })
     }
-    fn parse_cwd(&self, kdl_node: &KdlNode) -> Result<Option<PathBuf>, ConfigError> {
-        Ok(
-            kdl_get_string_property_or_child_value_with_error!(kdl_node, "cwd")
-                .map(|cwd| PathBuf::from(cwd)),
-        )
+    fn parse_path(
+        &self,
+        kdl_node: &KdlNode,
+        name: &'static str,
+    ) -> Result<Option<PathBuf>, ConfigError> {
+        match kdl_get_string_property_or_child_value_with_error!(kdl_node, name) {
+            Some(s) => match shellexpand::full(s) {
+                Ok(s) => Ok(Some(PathBuf::from(s.as_ref()))),
+                Err(e) => Err(kdl_parsing_error!(e.to_string(), kdl_node)),
+            },
+            None => Ok(None),
+        }
     }
     fn parse_pane_command(
         &self,
         pane_node: &KdlNode,
         is_template: bool,
     ) -> Result<Option<Run>, ConfigError> {
-        let command = kdl_get_string_property_or_child_value_with_error!(pane_node, "command")
-            .map(|c| PathBuf::from(c));
-        let edit = kdl_get_string_property_or_child_value_with_error!(pane_node, "edit")
-            .map(|c| PathBuf::from(c));
-        let cwd = self.parse_cwd(pane_node)?;
+        let command = self.parse_path(pane_node, "command")?;
+        let edit = self.parse_path(pane_node, "edit")?;
+        let cwd = self.parse_path(pane_node, "cwd")?;
         let args = self.parse_args(pane_node)?;
         let close_on_exit =
             kdl_get_bool_property_or_child_value_with_error!(pane_node, "close_on_exit");
@@ -1047,8 +1052,7 @@ impl<'a> KdlLayoutParser<'a> {
         self.assert_valid_tab_properties(kdl_node)?;
         let tab_name =
             kdl_get_string_property_or_child_value!(kdl_node, "name").map(|s| s.to_string());
-        let tab_cwd =
-            kdl_get_string_property_or_child_value!(kdl_node, "cwd").map(|c| PathBuf::from(c));
+        let tab_cwd = self.parse_path(kdl_node, "cwd")?;
         let is_focused = kdl_get_bool_property_or_child_value!(kdl_node, "focus").unwrap_or(false);
         let children_split_direction = self.parse_split_direction(kdl_node)?;
         let mut child_floating_panes = vec![];
@@ -1374,8 +1378,7 @@ impl<'a> KdlLayoutParser<'a> {
     ) -> Result<(), ConfigError> {
         let has_borderless_prop =
             kdl_get_bool_property_or_child_value_with_error!(kdl_node, "borderless").is_some();
-        let has_cwd_prop =
-            kdl_get_string_property_or_child_value_with_error!(kdl_node, "cwd").is_some();
+        let has_cwd_prop = self.parse_path(kdl_node, "cwd")?.is_some();
         let has_non_cwd_run_prop = self
             .parse_command_plugin_or_edit_block(kdl_node)?
             .map(|r| match r {
@@ -1445,8 +1448,7 @@ impl<'a> KdlLayoutParser<'a> {
         // (is_focused, Option<tab_name>, PaneLayout, Vec<FloatingPaneLayout>)
         let tab_name =
             kdl_get_string_property_or_child_value!(kdl_node, "name").map(|s| s.to_string());
-        let tab_cwd =
-            kdl_get_string_property_or_child_value!(kdl_node, "cwd").map(|c| PathBuf::from(c));
+        let tab_cwd = self.parse_path(kdl_node, "cwd")?;
         let is_focused = kdl_get_bool_property_or_child_value!(kdl_node, "focus").unwrap_or(false);
         let children_split_direction = self.parse_split_direction(kdl_node)?;
         match kdl_children_nodes!(kdl_node) {
@@ -1679,11 +1681,7 @@ impl<'a> KdlLayoutParser<'a> {
     fn populate_global_cwd(&mut self, layout_node: &KdlNode) -> Result<(), ConfigError> {
         // we only populate global cwd from the layout file if another wasn't explicitly passed to us
         if self.global_cwd.is_none() {
-            if let Some(global_cwd) =
-                kdl_get_string_property_or_child_value_with_error!(layout_node, "cwd")
-            {
-                self.global_cwd = Some(PathBuf::from(global_cwd));
-            }
+            self.global_cwd = self.parse_path(layout_node, "cwd")?;
         }
         Ok(())
     }


Test patch:
diff --git a/zellij-utils/src/input/unit/layout_test.rs b/zellij-utils/src/input/unit/layout_test.rs
index a38abe0287..fee88aae44 100644
--- a/zellij-utils/src/input/unit/layout_test.rs
+++ b/zellij-utils/src/input/unit/layout_test.rs
@@ -2071,3 +2071,60 @@ fn run_plugin_location_parsing() {
     };
     assert_eq!(layout, expected_layout);
 }
+
+#[test]
+fn env_var_expansion() {
+    let raw_layout = r#"
+        layout {
+            // cwd tests + composition
+            cwd "$TEST_GLOBAL_CWD"
+            pane cwd="relative"  // -> /abs/path/relative
+            pane cwd="/another/abs"  // -> /another/abs
+            pane cwd="$TEST_LOCAL_CWD"  // -> /another/abs
+            pane cwd="$TEST_RELATIVE"  // -> /abs/path/relative
+            pane command="ls" cwd="$TEST_ABSOLUTE"  // -> /somewhere
+            pane edit="file.rs" cwd="$TEST_ABSOLUTE"  // -> /somewhere/file.rs
+            pane edit="file.rs" cwd="~/backup"  // -> /home/aram/backup/file.rs
+
+            // other paths
+            pane command="~/backup/executable"  // -> /home/aram/backup/executable
+            pane edit="~/backup/foo.txt"  // -> /home/aram/backup/foo.txt
+        }
+    "#;
+    let env_vars = [
+        ("TEST_GLOBAL_CWD", "/abs/path"),
+        ("TEST_LOCAL_CWD", "/another/abs"),
+        ("TEST_RELATIVE", "relative"),
+        ("TEST_ABSOLUTE", "/somewhere"),
+        ("HOME", "/home/aram"),
+    ];
+    let mut old_vars = Vec::new();
+    // set environment variables for test, keeping track of existing values.
+    for (key, value) in env_vars {
+        old_vars.push((key, std::env::var(key).ok()));
+        std::env::set_var(key, value);
+    }
+    let layout = Layout::from_kdl(raw_layout, "layout_file_name".into(), None, None);
+    // restore environment.
+    for (key, opt) in old_vars {
+        match opt {
+            Some(value) => std::env::set_var(key, &value),
+            None => std::env::remove_var(key),
+        }
+    }
+    let layout = layout.unwrap();
+    assert_snapshot!(format!("{layout:#?}"));
+}
+
+#[test]
+fn env_var_missing() {
+    std::env::remove_var("SOME_UNIQUE_VALUE");
+    let kdl_layout = r#"
+        layout {
+            cwd "$SOME_UNIQUE_VALUE"
+            pane cwd="relative"
+        }
+    "#;
+    let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into(), None, None);
+    assert!(layout.is_err(), "invalid env var lookup should fail");
+}
diff --git a/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__env_var_expansion.snap b/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__env_var_expansion.snap
new file mode 100644
index 0000000000..de30d22961
--- /dev/null
+++ b/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__env_var_expansion.snap
@@ -0,0 +1,212 @@
+---
+source: zellij-utils/src/input/./unit/layout_test.rs
+assertion_line: 2116
+expression: "format!(\"{layout:#?}\")"
+---
+Layout {
+    tabs: [],
+    focused_tab_index: None,
+    template: Some(
+        (
+            TiledPaneLayout {
+                children_split_direction: Horizontal,
+                name: None,
+                children: [
+                    TiledPaneLayout {
+                        children_split_direction: Horizontal,
+                        name: None,
+                        children: [],
+                        split_size: None,
+                        run: Some(
+                            Cwd(
+                                "/abs/path/relative",
+                            ),
+                        ),
+                        borderless: false,
+                        focus: None,
+                        external_children_index: None,
+                        children_are_stacked: false,
+                        is_expanded_in_stack: false,
+                        exclude_from_sync: None,
+                    },
+                    TiledPaneLayout {
+                        children_split_direction: Horizontal,
+                        name: None,
+                        children: [],
+                        split_size: None,
+                        run: Some(
+                            Cwd(
+                                "/another/abs",
+                            ),
+                        ),
+                        borderless: false,
+                        focus: None,
+                        external_children_index: None,
+                        children_are_stacked: false,
+                        is_expanded_in_stack: false,
+                        exclude_from_sync: None,
+                    },
+                    TiledPaneLayout {
+                        children_split_direction: Horizontal,
+                        name: None,
+                        children: [],
+                        split_size: None,
+                        run: Some(
+                            Cwd(
+                                "/another/abs",
+                            ),
+                        ),
+                        borderless: false,
+                        focus: None,
+                        external_children_index: None,
+                        children_are_stacked: false,
+                        is_expanded_in_stack: false,
+                        exclude_from_sync: None,
+                    },
+                    TiledPaneLayout {
+                        children_split_direction: Horizontal,
+                        name: None,
+                        children: [],
+                        split_size: None,
+                        run: Some(
+                            Cwd(
+                                "/abs/path/relative",
+                            ),
+                        ),
+                        borderless: false,
+                        focus: None,
+                        external_children_index: None,
+                        children_are_stacked: false,
+                        is_expanded_in_stack: false,
+                        exclude_from_sync: None,
+                    },
+                    TiledPaneLayout {
+                        children_split_direction: Horizontal,
+                        name: None,
+                        children: [],
+                        split_size: None,
+                        run: Some(
+                            Command(
+                                RunCommand {
+                                    command: "ls",
+                                    args: [],
+                                    cwd: Some(
+                                        "/somewhere",
+                                    ),
+                                    hold_on_close: true,
+                                    hold_on_start: false,
+                                },
+                            ),
+                        ),
+                        borderless: false,
+                        focus: None,
+                        external_children_index: None,
+                        children_are_stacked: false,
+                        is_expanded_in_stack: false,
+                        exclude_from_sync: None,
+                    },
+                    TiledPaneLayout {
+                        children_split_direction: Horizontal,
+                        name: None,
+                        children: [],
+                        split_size: None,
+                        run: Some(
+                            EditFile(
+                                "/somewhere/file.rs",
+                                None,
+                                Some(
+                                    "/somewhere",
+                                ),
+                            ),
+                        ),
+                        borderless: false,
+                        focus: None,
+                        external_children_index: None,
+                        children_are_stacked: false,
+                        is_expanded_in_stack: false,
+                        exclude_from_sync: None,
+                    },
+                    TiledPaneLayout {
+                        children_split_direction: Horizontal,
+                        name: None,
+                        children: [],
+                        split_size: None,
+                        run: Some(
+                            EditFile(
+                                "/home/aram/backup/file.rs",
+                                None,
+                                Some(
+                                    "/home/aram/backup",
+                                ),
+                            ),
+                        ),
+                        borderless: false,
+                        focus: None,
+                        external_children_index: None,
+                        children_are_stacked: false,
+                        is_expanded_in_stack: false,
+                        exclude_from_sync: None,
+                    },
+                    TiledPaneLayout {
+                        children_split_direction: Horizontal,
+                        name: None,
+                        children: [],
+                        split_size: None,
+                        run: Some(
+                            Command(
+                                RunCommand {
+                                    command: "/home/aram/backup/executable",
+                                    args: [],
+                                    cwd: Some(
+                                        "/abs/path",
+                                    ),
+                                    hold_on_close: true,
+                                    hold_on_start: false,
+                                },
+                            ),
+                        ),
+                        borderless: false,
+                        focus: None,
+                        external_children_index: None,
+                        children_are_stacked: false,
+                        is_expanded_in_stack: false,
+                        exclude_from_sync: None,
+                    },
+                    TiledPaneLayout {
+                        children_split_direction: Horizontal,
+                        name: None,
+                        children: [],
+                        split_size: None,
+                        run: Some(
+                            EditFile(
+                                "/home/aram/backup/foo.txt",
+                                None,
+                                Some(
+                                    "/abs/path",
+                                ),
+                            ),
+                        ),
+                        borderless: false,
+                        focus: None,
+                        external_children_index: None,
+                        children_are_stacked: false,
+                        is_expanded_in_stack: false,
+                        exclude_from_sync: None,
+                    },
+                ],
+                split_size: None,
+                run: None,
+                borderless: false,
+                focus: None,
+                external_children_index: None,
+                children_are_stacked: false,
+                is_expanded_in_stack: false,
+                exclude_from_sync: None,
+            },
+            [],
+        ),
+    ),
+    swap_layouts: [],
+    swap_tiled_layouts: [],
+    swap_floating_layouts: [],
+}