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

Oil should have a slurp builtin #711

Closed
andychu opened this issue Apr 16, 2020 · 9 comments
Closed

Oil should have a slurp builtin #711

andychu opened this issue Apr 16, 2020 · 9 comments

Comments

@andychu
Copy link
Contributor

andychu commented Apr 16, 2020

Problems:

So slurp can be used as follows:

slurp myvar < myfile.txt  # includes newlines if any, status is 0

grep foo *.c | slurp myvar    # works because of shopt -s lastpipe

related:

@Crestwave
Copy link
Contributor

Crestwave commented Apr 17, 2020

So like:

while read -r line; do
    myvar+=$line$'\n'
done <myfile.txt
[[ -n $line ]] && myvar+=$line

But shorter and probably much faster as well, of course.

@akinomyoga
Copy link
Collaborator

Another solution with mapfile (which preserves the delimiter unless -t is specified):

mapfile myvar < myfile.txt
IFS= eval 'myvar="${myvar[*]}"'

If one can assume that there is no NUL byte (which cannot be handled properly in Bash) in the file, here is the implementation in ble.sh (ble/util/readfile). You can just write in a single line:

mapfile -d '' myvar < myfile.txt

Or, with read,

IFS= TMOUT= read -r -d '' myvar < myfile.txt || [[ $myvar ]]

@andychu
Copy link
Contributor Author

andychu commented Apr 17, 2020

Ah OK mapfile -d '' myvar is pretty simple.

And Oil supports NUL bytes, so that would work with either mapfile or slurp.

So I guess slurp is not strictly necessary then, although it's shorter. I suppose we can implement it as a "polyfill" shell function, like

@david-a-wheeler
Copy link

It might be easier to simply add another option or two to "read"; read is already (essentially) reserved. E.g., "-R" (raw) option would ignore IFS & not re-interpret anything, and simply load stdin's contents into the named variable (up to the byte length limits if given).

@andychu andychu added the stdlib label Apr 19, 2020
@andychu
Copy link
Contributor Author

andychu commented Apr 19, 2020

Yeah that is possible -- it's a matter of style. If that were the case I'd want to implement --long flags ( #465 ) first (at least the flag parser support).

Oil currently has:

  • getline, which is roughly like read -r, and could be read --line. Although somehow that seems long and awkward. Also read is unbuffered, but getline is buffered I/O which I think is also useful.
  • write to avoid the -- problem with echo

So it could be read --all instead of slurp. But I don't like that the common things require flags, while the uncommon thing doesn't. So that's why I wanted to "promote" these more common behaviors to their own builtins.

I added that principle here:

https://github.com/oilshell/oil/wiki/Language-Design-Principles

e.g. I believe read -r should have been the default, and is read is a bit error prone as is. And I think read -r vs read -R is too confusing without long flags.

Thanks for the feedback.

@andychu
Copy link
Contributor Author

andychu commented Apr 19, 2020

It is possibly safer to create read --line, and read --all, and then optional "aliases" getline and slurp.

But shells already stomp all over other commands like time, kill, printf, etc. And there is a mechanism to diable that -- command time, command slurp, etc.

@andychu
Copy link
Contributor Author

andychu commented Apr 19, 2020

This is related to #588, which is basically namespace management for the "first word".

And to some degree #453 for function namespaces.

andychu pushed a commit that referenced this issue Oct 11, 2020
@andychu
Copy link
Contributor Author

andychu commented Oct 11, 2020

OK we now have

read --line :myline   # optional : for variable names
echo "$myline"

read --all :x
echo "$x"

I have been getting feedback that it's better not to stomp on the "first word" namespace, so these are indeed long flags, rather than slurp and getline.

Feedback is welcome!

@andychu
Copy link
Contributor Author

andychu commented Oct 28, 2020

@andychu andychu closed this as completed Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants