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

IOBuffer of SubString #9888

Closed
hayd opened this issue Jan 23, 2015 · 11 comments
Closed

IOBuffer of SubString #9888

hayd opened this issue Jan 23, 2015 · 11 comments
Assignees
Labels
help wanted Indicates that a maintainer wants help on an issue or pull request io Involving the I/O subsystem: libuv, read, write, etc.

Comments

@hayd
Copy link
Member

hayd commented Jan 23, 2015

It seems like this might be useful to define, for example if you don't know whether the type is a string or a substring.

I had a go implemented with the following in this commit:

IOBuffer(s::SubString) = IOBuffer(s.string.data[s.offset:s.endof], true, false)

however, that seems not to work. :(

@ivarne
Copy link
Member

ivarne commented Jan 23, 2015

How doesn't it work?

@hayd
Copy link
Member Author

hayd commented Jan 23, 2015

@ivarne great question, I will try and come up with a simple example (was hoping it was somethng obvious). I think it's unicode related.

The bug was with unicode in markdown tables with the above definition (from the commit), without unicode it works (IIRC).

@ivarne
Copy link
Member

ivarne commented Jan 23, 2015

Looking at how the .endof and .offset fields are used leads me to think that

IOBuffer(s::SubString) = IOBuffer(s.string.data[(s.offset:(s.offset+s.endof)) + 1], true, false)

will work much better.

Edit: Changed - 1 to + 1, sorry if you look at the email!

@stevengj
Copy link
Member

That seems suboptimal because it is creating a copy of the data (at least until #9150 lands). And if you're willing to make a copy, you could just do IOBuffer(s::AbstractString) = IOBuffer(utf8(s)).

Doing this without making a copy seems problematic right now because IOBuffer requires an Array{Uint8}, and doesn't permit subarrays.

@ivarne
Copy link
Member

ivarne commented Jan 23, 2015

I just thought IOBuffer needed a real Array, but if it can work directly with a SubArray, it might be worth it.

@stevengj
Copy link
Member

It looks like the code would work with any AbstractVector supporting pointer and stride where the stride is 1 (i.e. contiguous). This should include contiguous subarrays, although I'm not sure what methods are defined for SubArray as I haven't used that type much.

@stevengj stevengj added the io Involving the I/O subsystem: libuv, read, write, etc. label Jan 23, 2015
@vtjnash
Copy link
Member

vtjnash commented Jan 26, 2015

i think we can make IOBuffer parameterized by the type of the AbstractVector{Uint8} that it contains, rather than forcing Vector{Uint8}. that would also allow it to wrap a SharedArray too.

@JeffBezanson
Copy link
Member

Doesn't IOBuffer kind of need to own the vector it wraps? I guess a read-only or non-growable IOBuffer on a SubArray would be ok though.

@stevengj
Copy link
Member

@JeffBezanson, in the IOBuffer(::String) case the buffer is read-only.

@vtjnash
Copy link
Member

vtjnash commented Jan 27, 2015

right. when creating a IOBuffer from an existing object, it should typically default to read-only

@vtjnash vtjnash added feature help wanted Indicates that a maintainer wants help on an issue or pull request labels Jan 27, 2015
@vtjnash vtjnash self-assigned this Mar 27, 2015
vtjnash added a commit that referenced this issue Jun 3, 2015
vtjnash added a commit that referenced this issue Jun 3, 2015
vtjnash added a commit that referenced this issue Jun 4, 2015
vtjnash added a commit that referenced this issue Jun 6, 2015
@vtjnash
Copy link
Member

vtjnash commented Jul 15, 2015

this was implemented in #11554

@vtjnash vtjnash closed this as completed Jul 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Indicates that a maintainer wants help on an issue or pull request io Involving the I/O subsystem: libuv, read, write, etc.
Projects
None yet
Development

No branches or pull requests

6 participants