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.

Sign up for GitHub

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

direnv: allow adding additional files to watch #182

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

direnv: allow adding additional files to watch #182

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/direnv.ncl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@
| Bool
| default
= true,
watch_files
| doc m%"
Reload the shell when the contents of these files change.
"%
| Array String
| default
= []
},
files =
if direnv.enable then
Expand All @@ -16,7 +23,7 @@
if ! has nix_direnv_version || ! nix_direnv_version 2.4.0; then
source_url "https://raw.githubusercontent.com/nix-community/nix-direnv/2.4.0/direnvrc" "sha256-XQzUAvL6pysIJnRJyR7uVpmUSZfc7LSgWQwq/4mBr1U="
fi
nix_direnv_watch_file project.ncl nickel.lock.ncl
nix_direnv_watch_file project.ncl nickel.lock.ncl %{std.string.join " " direnv.watch_files}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we escape the file names here, in all generality? For example:

let escape = std.string.replace "\"" "\\\"" in
let enclose = fun s => "\"%{s}\"" in
let extra_watch_files =
  direnv.watch_files
  |> std.array.map (fun s => s |> escape |> enclose)
  |> std.string.join " "
in
..
            nix_direnv_watch_file project.ncl nickel.lock.ncl extra_watch_files

Copy link
Author

@nrdxp nrdxp Dec 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming this is for and other special characters? Perhaps we should use single quotes (') in the enclose function to also avoid arbitrary command expansions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, ' is probably better (then we need to escape them instead of " in escape)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nrdxp are you still willing to apply this change? I think after that this PR is good to go!

source_env_if_exists ".envrc.private"
use flake
"%,
Expand Down