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.

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

Global justfile #1846

Merged
merged 34 commits into from
May 19, 2024
Merged

Global justfile #1846

merged 34 commits into from
May 19, 2024

Conversation

neunenak
Copy link
Contributor

Allow the -g or --global flag to reference a global justfile in the user's home directory.

Resolves #1648

@neunenak neunenak force-pushed the global-justfile branch 2 times, most recently from 13521ed to b1eed71 Compare January 14, 2024 10:41
@neunenak neunenak marked this pull request as ready for review January 14, 2024 10:55
@neunenak
Copy link
Contributor Author

One thing that's probably worth discussing is whether the default justfile path should be $HOME/justfile , $HOME/.justfile, or $XDG_CONFIG_DIR/just/justfile. Using bare dotfiles in the homedir seems like it's falling out of fashion these days, and for good reason, so really it's just a choice between $HOME/justfile and $XDG_CONFIG_DIR/just/justfile. Or if it should search for both (and in what order) and just use the first one it finds.

@casey
Copy link
Owner

casey commented Jan 15, 2024

I personally prefer dotfiles in my homedir, since it's easier to edit than the XDG config dir, which on a mac is in ~/Library/Application Support which is very verbose. Also, the cost of checking an additional location is minimal. So I'd do ~/.justfile, ~/justfile, and then $XDG_CONFIG_DIR/just/global.just, although I'm not sure in which order.

@neunenak
Copy link
Contributor Author

@casey this is ready for review

@casey
Copy link
Owner

casey commented May 15, 2024

Some notes:

  • I removed the ability to initialize the global justfile. I think it has a bug, where if the config directory doesn't exist, it will throw an I/O error.

  • This could use a test, which sets XDG_CONFIG_HOME to a tempdir with a justfile

  • What do you think about calling the global justfile in the config dir justfile instead of global.justfile?

  • Why not use dirs::config_dir() instead of looking up XDG_CONFIG_HOME manually?

@neunenak
Copy link
Contributor Author

Some notes:

* I removed the ability to initialize the global justfile. I think it has a bug, where if the config directory doesn't exist, it will throw an I/O error.

Might be nice to have the ability to initialize a global justfile, but that's additional functionality that can come in a subsequent PR.

* This could use a test, which sets XDG_CONFIG_HOME to a tempdir with a justfile

Makes sense, I'll try to add this shortly (probably not tonight).

* What do you think about calling the global justfile in the config dir `justfile` instead of `global.justfile`?

Fine by me.

* Why not use `dirs::config_dir()` instead of looking up `XDG_CONFIG_HOME` manually?

Ah I didn't realize there was already code that this.

@neunenak
Copy link
Contributor Author

@casey looks like the macos CI job is failing with no error message. I'm not sure if this is a problem with these changes or CI.

@neunenak
Copy link
Contributor Author

@casey looks like the macos CI job is failing with no error message. I'm not sure if this is a problem with these changes or CI.

I think what I figured out was that setting XDG_CONFIG_DIR in the test for this functionality only works on linux, so I disabled the test for other OSs. Not sure if there's a way to fake the configuration directory for those OSs.

@casey
Copy link
Owner

casey commented May 17, 2024

I think what I figured out was that setting XDG_CONFIG_DIR in the test for this functionality only works on linux, so I disabled the test for other OSs. Not sure if there's a way to fake the configuration directory for those OSs.

I think it's fine not to test other OSs, since the only way something could go wrong is if there was a bug in dirs.

@casey casey enabled auto-merge (squash) May 19, 2024 09:25
@casey
Copy link
Owner

casey commented May 19, 2024

LGTM! I made the flag --global-justfile, since there's already -g for short, and --global seemed ambiguous. I also made it conflict with --justfile and --working-directory. Support for --working-directory can be added in a future PR, since they could work together.

@casey casey merged commit a343f5c into casey:master May 19, 2024
5 checks passed
@neunenak neunenak deleted the global-justfile branch May 19, 2024 09:29
neunenak added a commit to neunenak/just that referenced this pull request May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for global justfiles
2 participants