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

Coord.abs off by one on Coord.nextLine; position in CoordinatedStream off-by-one #3

Closed
fsieczkowski opened this issue Aug 30, 2013 · 2 comments

Comments

@fsieczkowski
Copy link
Member

This issue consists of two separate, small bugs.

Firstly, the way Coord.nextLine is implemented, we get
Coord.eq (c, Coord.nextLine c) = true
which probably shouldn't be the case. This is because Coord.nextLine does not increment the abs counter of Coord.coord, and equality check only compares the absolute counts. This also makes the actual absolute position diverge more and more, which is annoying (if easy to work around) if trying to, say, interface with emacs.

The second issue is a really small thing about CoordinatedStream. Right now, the character in the returned stream is coordinated with the "new" value, coord' — in particular, end-of-line characters are associated with the next line, and the column positions are off by one, since the first column is taken up by the eols.

I will probably submit a patch later today.

fsieczkowski pushed a commit to fsieczkowski/cmlib that referenced this issue Aug 30, 2013
@robsimmons
Copy link
Member

Finally thinking about this today (sorry for the ... years). The coord absolute value problem is just a bug and I fixed just that.

Coordinated stream's design was intentional but probably bad. It was motivated by the idea that you might be dealing with partial inputs: you might have a stream that runs out, and then you want to have a new stream that picks up where it leaves off. Think about a REPL where you're only getting a line of multline input at a time. You need a way to track the position after the last character if you want to do this, and coordstream was designed with the "off by one error" with this in mind. Honestly, what this probably means is that coordstream too special purpose to be in CMlib.

@kcrary
Copy link
Contributor

kcrary commented Apr 3, 2021

I'm in agreement with Rob that Coord is too special-purpose for the library. I'm deprecating it in 71ee7.

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

No branches or pull requests

3 participants