On Thu, 2010-04-15 at 17:59 +0200, Stefan Sperling wrote:
> On Thu, Apr 15, 2010 at 11:28:13AM -0400, Greg Stein wrote:
> > I'd suggest that we simply remove mark/seek/reset from the translated stream.
>
> We could not handle keywords or newlines during patching if we did that.
Or, more accurately, we'd have to handle them a different way and that
would require substantial changes. But I think mark and seek
functionality is a natural thing for streams to have, and the idea of
seekable and non-seekable streams already has common precedent in other
systems.
(Reset can be seen as a special case of seek. I think it would be
better to just have a special mark value to pass to "seek" rather than
have a separate API, because that would help to emphasize and ensure
that it really does behave the same as seek, but that's not a big deal.)
> > (I was never comfortable with that extension to our streams in the
> > first place, and I don't see a reason that all streams must implement
> > them)
>
> What is the alternative?
>
> I used streams because the general trend was to go away from passing
> around naked APR file handles. I had to add some features to streams
> to use them for what I wanted to use them. So streams are used for
> more and more things and the original abstraction is being bent.
>
> I don't really mind which way it's done, but we need some way of
> supporting the features we want to support.
> For patch, that means either using streams that support reading
> ranges from a file, support transformation on lines read from
> the stream, and have mark/seek/reset, or using naked APR
> files and passing around file handles and range information.
>
> Realistically speaking, at this stage in the development cycle
> I'd much rather fix the mark/seek/reset support for translated
> streams than rewrite a lot of svn patch.
+1 to fixing and keeping mark/seek.
As for the other bits...
Reading a range from an APR file seems too specialized for a public API;
can we make it private for now?
The "line filter" and "line transformer": I don't think these fit the
stream abstraction at all well. Streams don't have "lines" as a primary
concept. Without going into the details, I think it can't play nicely
with plain reads and writes, and I doubt it's properly integrated with
mark/seek. I think line filtering should be implemented as a wrapper
stream instead.
- Julian
> And I guess some people won't be happy if we make patch use raw
> files instead of streams (e.g. Bert suggested the public patch API
> should accept a stream to read the patch from, rather than accept
> an APR file as it does now).
>
> Stefan
Received on 2010-04-15 18:29:55 CEST