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

Implement loading justfile from stdin #1933

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

drzymalanet
Copy link

I think it would be nice to have the possibility to pipe justfiles into just, like so:

cat input.txt | just --dump --justfile - > output.txt

It would work by detecting a special symbol "-" as the -f / --justfile argument.

Benefits:

  • Some code editors like helix require this in order to make auto formatting to work,
  • Many command line tools work like this and it seems it will not hurt to have,

The plan:

  1. Move Search{} functionality into Source{}
  2. Implement Source::from_stdin()
  3. Add "-" detection to the argument parser.

Here is the first of the commits, more commits will be added while work progresses.

@casey
Copy link
Owner

casey commented May 15, 2024

This seems reasonable, but this PR looks a bit too complicated and I think some of the changes are unnecessary.

I would create two new SearchConfig variants, WithStdin, and WithStdinAndWorkingDirectory, and handle those two cases separately in Search::find.

@drzymalanet drzymalanet force-pushed the load-from-stdin branch 2 times, most recently from 522568c to 2b3bed9 Compare May 15, 2024 12:03
@drzymalanet
Copy link
Author

@casey I have added the variants, now we need to change Search.justfile type because it is a PathBuf which does not fit to the stdin stream. It seems to me that changing it to an Rc<dyn std::io::Read> or a custom JustfileSource would be some options. What do you think?

@casey
Copy link
Owner

casey commented May 16, 2024

There are two options I can think of:

  1. Add an additional field to Search, which is a string containing the loaded source
  2. Make Search::justfile an enum, like JustfileSource

I would try out 1 first and see how it looks and see if it causes any problems, or requires a big refactor.

@casey casey marked this pull request as draft May 25, 2024 07:32
@drzymalanet drzymalanet reopened this Jun 11, 2024
@drzymalanet
Copy link
Author

Sorry for closing and reopening. It was by mistake.

@casey Please have a look at the new commit. I have added a JustfileKind enum to the Search struct. It does not feel right, but I have no idea how to make it better without a bigger refactor. Let me know what you think.

@casey
Copy link
Owner

casey commented Jun 13, 2024

This looks like a good approach to me!

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.

None yet

2 participants