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

read-upto command is not documented #938

Open
krader1961 opened this issue Mar 11, 2020 · 2 comments
Open

read-upto command is not documented #938

krader1961 opened this issue Mar 11, 2020 · 2 comments

Comments

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Mar 11, 2020

The read-upto command introduced by commit 770904b on 2019-12-30 is not documented. I'm opening this issue rather than creating a PR to document the command because it's not entirely clear what the desired behavior is. The original issue, that resulted in this command being created, was clearly talking about handling ASCII compatible char encodings. Including UTF-8 but not necessarily other encodings such as EUC-CN for the GB 2312 character set.

Before documenting read-upto I think there needs to be a longer discussion about what we want such a command to do. My opinion is that there should be a read-line that reads a line from the byte stream and omits the terminating char(s). That is, whether on MS Windows or UNIX it reads a line and returns the result without the \cr\lf or \lf terminator. There might be a use for something like the current read-upto but none of the existing examples seem to be convincing. It looks like all of them can be handled by reading all of the byte input, or just a single line, then using commands to parse the input on something like the ASCII RS char would work just as well.

@zzamboni

This comment has been minimized.

Copy link
Contributor

@zzamboni zzamboni commented Mar 11, 2020

@krader1961 the original use case from #831 cannot be solved by reading all/one line of input, since the protocol used by gitstatusd is not newline-separated.

The suggestion to implement a read-line function has been discussed, but @xiaq at the time indicated he thought it was not necessary since it's a trivial wrapper around read-upto (sorry for the screenshot, I don't know how to link to Telegram conversations):
image

@krader1961

This comment has been minimized.

Copy link
Contributor Author

@krader1961 krader1961 commented Mar 12, 2020

the original use case from #831 cannot be solved by reading all/one line of input, since the protocol used by gitstatusd is not newline-separated.

It appears to use ASCII RS (0x1E) the "record separator" char where newline would typically be used, and US (0x1F) the "unit separator" where a tab or comma would typically be used. Presumably to allow the values of interest to contain those characters. It uses a compact "record" structure but that's not really important. Simply slurping all its output and then splitting it on those control chars would work just as well as reading up to each control char, AFAICT. And that appears to be what https://github.com/romkatv/gitstatus/blob/master/gitstatus.plugin.zsh does.

The problem with read-upto is that it can't be used to read lines, in a portable fashion, if you care that a bare newline on Windows does not terminate a line. Even if that isn't a problem in practice, or you by fiat say that is malformed input, you still have the problem that your elvish script can't just trim the trailing newline. It also has to decide if it's on Windows and therefore also needs to trim the trailing carriage-return.

So I'm not opposed to retaining, and documenting, read-upto but it still seems like a dedicated read-line command is also warranted. Even if it's implemented as a function that wraps read-upto in an OS portable manner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.