You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <eh...@gmail.com> on 2005/11/27 21:45:19 UTC
[PATCH] Add a keyword+eol translating stream
lundblad suggested I change blame not to use the extra intermediate
file I made it use for certain EOL style settings. In order to do so,
I need a diff implementation which runs on streams (I'm working on
it!), and the patch below, which translates a stream on-the-fly.
I'm hoping for comments.
bye,
Erik.
Log:
[[[
Prepare to eliminate the - recently introduced - intermediate file for blame.
To prevent the creation of an extra intermediate file, on the fly recoding
of EOLs is required. This stream class provides that.
* include/svn_subst.h
* libsvn_subr/subst.c
(svn_subst_stream_translated): New. Returns a read/write stream
doing keyword and eol translation, proxying another stream.
* libsvn_subr/subst.c
(translate_chunk): New. Abstracted from svn_subst_translate_stream3,
adapted to use the 'const char' buffer argument required for write
stream translation.
(translation_baton): New. Structure for use with translate_chunk
to store intermediate state.
(create_translation_baton): New local function to create and initialize
a translation_baton.
(translated_stream_baton): New. Baton to hold values associated with
a translating stream.
(translated_stream_read, translated_stream_write,
translated_stream_close): New. Stream callbacks for translated streams.
(svn_subst_translate_stream3): Reimplement using translate_chunk().
(svn_subst_copy_and_translate3): Reimplement using the new stream
and svn_stream_copy(), primarily to excersize the code path.
]]]
Index: subversion/include/svn_subst.h
===================================================================
--- subversion/include/svn_subst.h (revision 17535)
+++ subversion/include/svn_subst.h (working copy)
@@ -227,6 +227,19 @@
svn_boolean_t expand,
apr_pool_t *pool);
+/** Return a stream which performs eol translation and keyword
+ * expansion much like svn_subst_translate_stream3() except that
+ * it's done on-the-fly when reading or writing to @a stream.
+ *
+ * @since New in 1.4
+ */
+svn_stream_t *
+svn_subst_stream_translated (svn_stream_t *stream,
+ const char *eol_str,
+ svn_boolean_t repair,
+ apr_hash_t *keywords,
+ svn_boolean_t expand);
+
/** Similar to svn_subst_translate_stream3() except relies upon a
* @c svn_subst_keywords_t struct instead of a hash for the keywords.
*
Index: subversion/libsvn_subr/subst.c
===================================================================
--- subversion/libsvn_subr/subst.c (revision 17535)
+++ subversion/libsvn_subr/subst.c (working copy)
@@ -823,141 +823,354 @@
return svn_subst_translate_stream3 (s, d, eol_str, repair, kh,
expand, pool); }
-svn_error_t *
-svn_subst_translate_stream3 (svn_stream_t *s, /* src stream */
- svn_stream_t *d, /* dst stream */
- const char *eol_str,
- svn_boolean_t repair,
- apr_hash_t *keywords,
- svn_boolean_t expand,
- apr_pool_t *pool)
+struct translation_baton
{
- char *buf;
- const char *p, *interesting;
- apr_size_t len, readlen;
- apr_size_t eol_str_len = eol_str ? strlen (eol_str) : 0;
- char newline_buf[2] = { 0 };
- apr_size_t newline_off = 0;
- char keyword_buf[SVN_KEYWORD_MAX_LEN] = { 0 };
- apr_size_t keyword_off = 0;
- char src_format[2] = { 0 };
- apr_size_t src_format_len = 0;
+ const char *eol_str;
+ svn_boolean_t repair;
+ apr_hash_t *keywords;
+ svn_boolean_t expand;
+ const char *interesting;
+ apr_size_t eol_str_len;
+ char newline_buf[2];
+ apr_size_t newline_off;
+ char keyword_buf[SVN_KEYWORD_MAX_LEN];
+ apr_size_t keyword_off;
+ char src_format[2];
+ apr_size_t src_format_len;
+} translation_baton;
- buf = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE + 1);
+/* Allocate a baton for use with translate_chunk() in POOL and
+ * initialize it for the first iteration.
+ *
+ * The caller must assure that EOL_STR and KEYWORDS are either
+ * allocated in POOL, or at least have the same guaranteed life time
+ * as the baton created.
+ *
+ */
+
+static struct translation_baton *
+create_translation_baton (const char *eol_str,
+ svn_boolean_t repair,
+ apr_hash_t *keywords,
+ svn_boolean_t expand,
+ apr_pool_t *pool)
+{
+ struct translation_baton *b = apr_palloc (pool, sizeof (*b));
+ int i;
+
/* For efficiency, convert an empty set of keywords to NULL. */
if (keywords && (apr_hash_count (keywords) == 0))
keywords = NULL;
- /* The docstring requires that *some* translation be requested. */
- assert (eol_str || keywords);
- interesting = (eol_str && keywords) ? "$\r\n" : eol_str ? "\r\n" : "$";
+ b->eol_str = eol_str;
+ b->eol_str_len = eol_str ? strlen (eol_str) : 0;
+ b->repair = repair;
+ b->keywords = keywords;
+ b->expand = expand;
+ b->interesting = (eol_str && keywords) ? "$\r\n" : eol_str ? "\r\n" : "$";
+ b->newline_buf[0] = '\0';
+ b->newline_buf[1] = '\0';
+ b->newline_off = 0;
+ for (i = 0; i < SVN_KEYWORD_MAX_LEN; i++)
+ b->keyword_buf[i] = '\0';
+ b->keyword_off = 0;
+ b->src_format[0] = '\0';
+ b->src_format[1] = '\0';
+ b->src_format_len = 0;
- readlen = SVN_STREAM_CHUNK_SIZE;
- while (readlen == SVN_STREAM_CHUNK_SIZE)
+ return b;
+}
+
+
+static svn_error_t *
+translate_chunk (svn_stream_t *d,
+ struct translation_baton *b,
+ const char *buf,
+ apr_size_t readlen,
+ apr_pool_t *pool)
+{
+ const char *p;
+ apr_size_t len;
+
+ if (buf)
{
- SVN_ERR (svn_stream_read (s, buf, &readlen));
+ /* precalculate some oft-used values */
+ const char *end = buf + readlen;
+ const char *interesting = b->interesting;
- buf[readlen] = '\0';
-
/* At the beginning of this loop, assume that we might be in an
* interesting state, i.e. with data in the newline or keyword
* buffer. First try to get to the boring state so we can copy
* a run of boring characters; then try to get back to the
* interesting state by processing an interesting character,
* and repeat. */
- for (p = buf; p < buf + readlen;)
+ for (p = buf; p < end;)
{
/* Try to get to the boring state, if necessary. */
- if (newline_off)
+ if (b->newline_off)
{
if (*p == '\n')
- newline_buf[newline_off++] = *p++;
+ b->newline_buf[b->newline_off++] = *p++;
- SVN_ERR (translate_newline (eol_str, eol_str_len, src_format,
- &src_format_len, newline_buf,
- newline_off, d, repair));
+ SVN_ERR (translate_newline (b->eol_str, b->eol_str_len,
+ b->src_format,
+ &b->src_format_len, b->newline_buf,
+ b->newline_off, d, b->repair));
- newline_off = 0;
+ b->newline_off = 0;
}
- else if (keyword_off && *p == '$')
+ else if (b->keyword_off && *p == '$')
{
/* If translation fails, treat this '$' as a starting '$'. */
- keyword_buf[keyword_off++] = '$';
- if (translate_keyword (keyword_buf, &keyword_off, expand,
- keywords))
+ b->keyword_buf[b->keyword_off++] = '$';
+ if (translate_keyword (b->keyword_buf, &b->keyword_off,
+ b->expand, b->keywords))
p++;
else
- keyword_off--;
+ b->keyword_off--;
- SVN_ERR (translate_write (d, keyword_buf, keyword_off));
+ SVN_ERR (translate_write (d, b->keyword_buf, b->keyword_off));
- keyword_off = 0;
+ b->keyword_off = 0;
}
- else if (keyword_off == SVN_KEYWORD_MAX_LEN - 1
- || (keyword_off && (*p == '\r' || *p == '\n')))
+ else if (b->keyword_off == SVN_KEYWORD_MAX_LEN - 1
+ || (b->keyword_off && (*p == '\r' || *p == '\n')))
{
/* No closing '$' found; flush the keyword buffer. */
- SVN_ERR (translate_write (d, keyword_buf, keyword_off));
-
- keyword_off = 0;
+ SVN_ERR (translate_write (d, b->keyword_buf, b->keyword_off));
+
+ b->keyword_off = 0;
}
- else if (keyword_off)
+ else if (b->keyword_off)
{
- keyword_buf[keyword_off++] = *p++;
+ b->keyword_buf[b->keyword_off++] = *p++;
continue;
}
- /* We're in the boring state; look for interest characters.
- * For lack of a memcspn(), manually skip past NULs. */
+ /* We're in the boring state; look for interest characters. */
len = 0;
- while (1)
- {
- len += strcspn (p + len, interesting);
- if (p[len] != '\0' || p + len == buf + readlen)
- break;
- len++;
- }
+
+ /* Fake strcspn() with a length parameter, strncspn if you like,
+ because we're not sure the buffer ends with a NUL character.
+
+ Also, reject NUL characters explicitly, since index()
+ considers them part of the string argument, but we don't...
+ */
+ while ((p + len) < end
+ && (! p[len] || ! index (interesting, p[len])))
+ len++;
+
if (len)
SVN_ERR (translate_write (d, p, len));
-
+
p += len;
/* Set up state according to the interesting character, if any. */
- switch (*p)
+ if (p < end)
{
- case '$':
- keyword_buf[keyword_off++] = *p++;
- break;
- case '\r':
- newline_buf[newline_off++] = *p++;
- break;
- case '\n':
- newline_buf[newline_off++] = *p++;
+ switch (*p)
+ {
+ case '$':
+ b->keyword_buf[b->keyword_off++] = *p++;
+ break;
+ case '\r':
+ b->newline_buf[b->newline_off++] = *p++;
+ break;
+ case '\n':
+ b->newline_buf[b->newline_off++] = *p++;
- SVN_ERR (translate_newline (eol_str, eol_str_len, src_format,
- &src_format_len, newline_buf,
- newline_off, d, repair));
+ SVN_ERR (translate_newline (b->eol_str, b->eol_str_len,
+ b->src_format,
+ &b->src_format_len,
+ b->newline_buf,
+ b->newline_off, d, b->repair));
- newline_off = 0;
- break;
+ b->newline_off = 0;
+ break;
+
+ }
}
}
}
+ else
+ {
+ if (b->newline_off)
+ SVN_ERR (translate_newline (b->eol_str, b->eol_str_len, b->src_format,
+ &b->src_format_len, b->newline_buf,
+ b->newline_off, d, b->repair));
- if (newline_off)
- SVN_ERR (translate_newline (eol_str, eol_str_len, src_format,
- &src_format_len, newline_buf, newline_off, d,
- repair));
+ if (b->keyword_off)
+ SVN_ERR (translate_write (d, b->keyword_buf, b->keyword_off));
+ }
- if (keyword_off)
- SVN_ERR (translate_write (d, keyword_buf, keyword_off));
+ return SVN_NO_ERROR;
+}
+struct translated_stream_baton
+{
+ svn_stream_t *stream;
+ struct translation_baton *in_baton, *out_baton;
+ svn_boolean_t written;
+ svn_stringbuf_t *readbuf;
+ char *buf;
+ apr_size_t readbuf_off;
+ apr_pool_t *pool;
+} translated_stream_baton;
+
+
+static svn_error_t *
+translated_stream_read (void *baton,
+ char *buffer,
+ apr_size_t *len)
+{
+ struct translated_stream_baton *b = baton;
+ apr_size_t readlen = SVN_STREAM_CHUNK_SIZE;
+ apr_size_t unsatisfied = *len;
+ apr_size_t off = 0;
+ apr_pool_t *iterpool;
+ svn_pool_clear (b->pool);
+
+ iterpool = svn_pool_create (b->pool);
+ while (readlen == SVN_STREAM_CHUNK_SIZE && unsatisfied > 0)
+ {
+ apr_size_t to_copy;
+
+ /* fill read buffer, if necessary */
+ if (! (b->readbuf_off < b->readbuf->len))
+ {
+ svn_stream_t *buf_stream;
+
+ svn_stringbuf_setempty (b->readbuf);
+ SVN_ERR (svn_stream_read (b->stream, b->buf, &readlen));
+ buf_stream = svn_stream_from_stringbuf (b->readbuf, iterpool);
+
+ SVN_ERR (translate_chunk (buf_stream, b->in_baton, b->buf,
+ readlen, iterpool));
+
+ if (readlen != SVN_STREAM_CHUNK_SIZE)
+ SVN_ERR (translate_chunk (buf_stream, b->in_baton, NULL, 0,
+ iterpool));
+
+ SVN_ERR (svn_stream_close (buf_stream));
+ }
+
+ /* Satisfy from the read buffer */
+ to_copy = (b->readbuf->len > unsatisfied)
+ ? unsatisfied : b->readbuf->len;
+ memcpy (buffer + off, b->readbuf->data, to_copy);
+ off += to_copy;
+ b->readbuf_off += to_copy;
+ unsatisfied -= to_copy;
+ }
+
+ *len -= unsatisfied;
+
return SVN_NO_ERROR;
}
+static svn_error_t *
+translated_stream_write (void *baton,
+ const char *buffer,
+ apr_size_t *len)
+{
+ struct translated_stream_baton *b = baton;
+ svn_pool_clear (b->pool);
+ b->written = TRUE;
+ SVN_ERR (translate_chunk (b->stream, b->out_baton, buffer, *len, b->pool));
+
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+translated_stream_close (void *baton)
+{
+ struct translated_stream_baton *b = baton;
+
+ if (b->written)
+ SVN_ERR (translate_chunk (b->stream, b->out_baton, NULL, 0, b->pool));
+
+ svn_pool_destroy (b->pool);
+
+ SVN_ERR (svn_stream_close (b->stream));
+ return SVN_NO_ERROR;
+}
+
+
+svn_stream_t *
+svn_subst_stream_translated (svn_stream_t *stream,
+ const char *eol_str,
+ svn_boolean_t repair,
+ apr_hash_t *keywords,
+ svn_boolean_t expand,
+ apr_pool_t *pool)
+{
+ struct translated_stream_baton *baton = apr_palloc (pool, sizeof (*baton));
+ svn_stream_t *s = svn_stream_create (baton, pool);
+
+ /* Make sure EOL_STR and KEYWORDS are allocated in POOL, as
+ required by create_translation_baton() */
+ if (eol_str)
+ eol_str = apr_pstrdup (pool, eol_str);
+ if (keywords)
+ keywords = apr_hash_copy (pool, keywords);
+
+ /* Setup the baton fields */
+ baton->stream = stream;
+ baton->in_baton
+ = create_translation_baton (eol_str, repair, keywords, expand, pool);
+ baton->out_baton
+ = create_translation_baton (eol_str, repair, keywords, expand, pool);
+ baton->written = FALSE;
+ baton->readbuf = svn_stringbuf_create ("", pool);
+ baton->readbuf_off = 0;
+ baton->buf = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE + 1);
+ baton->pool = svn_pool_create (pool);
+
+ /* Setup the stream methods */
+ svn_stream_set_read (s, translated_stream_read);
+ svn_stream_set_write (s, translated_stream_write);
+ svn_stream_set_close (s, translated_stream_close);
+
+ return s;
+}
+
+
svn_error_t *
+svn_subst_translate_stream3 (svn_stream_t *s, /* src stream */
+ svn_stream_t *d, /* dst stream */
+ const char *eol_str,
+ svn_boolean_t repair,
+ apr_hash_t *keywords,
+ svn_boolean_t expand,
+ apr_pool_t *pool)
+{
+ apr_pool_t *subpool = svn_pool_create (pool);
+ apr_pool_t *iterpool = svn_pool_create (subpool);
+ struct translation_baton *baton;
+ apr_size_t readlen = SVN_STREAM_CHUNK_SIZE;
+ char *buf = apr_palloc (subpool, SVN_STREAM_CHUNK_SIZE);
+
+ /* The docstring requires that *some* translation be requested. */
+ assert (eol_str || keywords);
+
+ baton = create_translation_baton (eol_str, repair, keywords, expand, pool);
+ while (readlen == SVN_STREAM_CHUNK_SIZE)
+ {
+ svn_pool_clear (iterpool);
+ SVN_ERR (svn_stream_read (s, buf, &readlen));
+ SVN_ERR (translate_chunk (d, baton, buf, readlen, iterpool));
+ }
+
+ SVN_ERR (translate_chunk (d, baton, NULL, 0, iterpool));
+
+ svn_pool_destroy (subpool); /* also destroys iterpool */
+ return SVN_NO_ERROR;
+}
+
+
+svn_error_t *
svn_subst_translate_stream (svn_stream_t *s, /* src stream */
svn_stream_t *d, /* dst stream */
const char *eol_str,
@@ -1256,9 +1469,19 @@
src_stream = svn_stream_from_aprfile (s, pool);
dst_stream = svn_stream_from_aprfile (d, pool);
+ /* No reason to choose either source or destination,
+ other than to exercise all code paths */
+ if (expand)
+ src_stream = svn_subst_stream_translated (src_stream,
+ eol_str, repair,
+ keywords, expand, pool);
+ else
+ dst_stream = svn_subst_stream_translated (dst_stream,
+ eol_str, repair,
+ keywords, expand, pool);
+
/* Translate src stream into dst stream. */
- err = svn_subst_translate_stream3 (src_stream, dst_stream, eol_str,
- repair, keywords, expand, pool);
+ err = svn_stream_copy (src_stream, dst_stream, pool);
if (err)
{
if (err->apr_err == SVN_ERR_IO_INCONSISTENT_EOL)
@@ -1266,6 +1489,8 @@
(SVN_ERR_IO_INCONSISTENT_EOL, err,
_("File '%s' has inconsistent newlines"),
svn_path_local_style (src, pool));
+ else
+ return err;
}
/* clean up nicely. */
Re: [PATCH] Add a keyword+eol translating stream
Posted by Erik Huelsmann <eh...@gmail.com>.
> >> + /* No reason to choose either source or destination,
> >> + other than to exercise all code paths */
> >> + if (expand)
> >> + src_stream = svn_subst_stream_translated (src_stream,
> >> + eol_str, repair,
> >> + keywords, expand, pool);
> >> + else
> >> + dst_stream = svn_subst_stream_translated (dst_stream,
> >> + eol_str, repair,
> >> + keywords, expand, pool);
> >
> > It's not this function's job to exercise your new code paths! By all
> > means try this as a form of regression test but, for committing to the
> > repository, if you can make this function simpler by cutting out the
> > "if" and one of the paths, please do, although it seems to me that it
> > would be even simpler, and thus better, if you just left it how it was.
> > Presumably you will soon add some code that really needs the translated
> > stream, and that will exercise it.
>
> What did you think of this? Not much, it appears! (If you've replied, I
> haven't received it.)
Only after lundblad sent his review of the actual commit, I understand
what you're referring to here! :-(
Actually: I removed that change (which, lundblad, is why it's not in
the log!). I'll revert that part. Though I'd actually prefer to
deprecate svn_subst_translate_streamX() instead, since the same can be
accomplished through this stream... (but I didn't - and still don't -
want to get into that discussion; it's just not worth it.)
bye,
Erik.
Re: [PATCH] Add a keyword+eol translating stream
Posted by Julian Foad <ju...@btopenworld.com>.
Julian Foad wrote:
> Erik Huelsmann wrote:
>
>> I'm hoping for comments.
>
> +1 in principle. I haven't got time to review it this week. I just had
> a quick browse through the diff and made a few comments.
I see you heeded most of my comments before committing r17559; thanks.
>> + /* No reason to choose either source or destination,
>> + other than to exercise all code paths */
>> + if (expand)
>> + src_stream = svn_subst_stream_translated (src_stream,
>> + eol_str, repair,
>> + keywords, expand, pool);
>> + else
>> + dst_stream = svn_subst_stream_translated (dst_stream,
>> + eol_str, repair,
>> + keywords, expand, pool);
>
> It's not this function's job to exercise your new code paths! By all
> means try this as a form of regression test but, for committing to the
> repository, if you can make this function simpler by cutting out the
> "if" and one of the paths, please do, although it seems to me that it
> would be even simpler, and thus better, if you just left it how it was.
> Presumably you will soon add some code that really needs the translated
> stream, and that will exercise it.
What did you think of this? Not much, it appears! (If you've replied, I
haven't received it.)
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Add a keyword+eol translating stream
Posted by Julian Foad <ju...@btopenworld.com>.
Julian Foad wrote:
> Erik Huelsmann wrote:
>
>> I'm hoping for comments.
>
> +1 in principle. I haven't got time to review it this week. I just had
> a quick browse through the diff and made a few comments.
Now you've committed it I noticed another one:
> +struct translation_baton
> {
> + const char *eol_str;
> + svn_boolean_t repair;
> + apr_hash_t *keywords;
> + svn_boolean_t expand;
> + const char *interesting;
> + apr_size_t eol_str_len;
> + char newline_buf[2];
> + apr_size_t newline_off;
> + char keyword_buf[SVN_KEYWORD_MAX_LEN];
> + apr_size_t keyword_off;
> + char src_format[2];
> + apr_size_t src_format_len;
> +} translation_baton;
I think that mention of "translation_baton" at the end declares a variable that
isn't used.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Add a keyword+eol translating stream
Posted by Julian Foad <ju...@btopenworld.com>.
Erik Huelsmann wrote:
> lundblad suggested I change blame not to use the extra intermediate
> file I made it use for certain EOL style settings. In order to do so,
> I need a diff implementation which runs on streams (I'm working on
> it!), and the patch below, which translates a stream on-the-fly.
>
> I'm hoping for comments.
+1 in principle. I haven't got time to review it this week. I just had a
quick browse through the diff and made a few comments.
> [[[
> Prepare to eliminate the - recently introduced - intermediate file for blame.
>
> To prevent the creation of an extra intermediate file, on the fly recoding
> of EOLs is required. This stream class provides that.
That mostly talks about your plans for "blame". Please could the log message
first and foremost say what the patch does, and just mention the intended use
for it briefly afterwards as a justification?
> Index: subversion/include/svn_subst.h
> ===================================================================
[...]
> +/** Return a stream which performs eol translation and keyword
> + * expansion much like svn_subst_translate_stream3() except that
> + * it's done on-the-fly when reading or writing to @a stream.
Could you remove "much" to make it more definite?
It might be worth saying explicitly that it does the same translation on
reading as on writing, not the opposite translation. It may be impossible to
do otherwise, with the given parameters, but I found that a little confusing at
first, wondering how it could know which way to translate.
> + *
> + * @since New in 1.4
> + */
> +svn_stream_t *
> +svn_subst_stream_translated (svn_stream_t *stream,
> + const char *eol_str,
> + svn_boolean_t repair,
> + apr_hash_t *keywords,
> + svn_boolean_t expand);
> Index: subversion/libsvn_subr/subst.c
> ===================================================================
[...]
> +svn_subst_stream_translated (svn_stream_t *stream,
> + const char *eol_str,
> + svn_boolean_t repair,
> + apr_hash_t *keywords,
> + svn_boolean_t expand,
> + apr_pool_t *pool)
> +{
> + struct translated_stream_baton *baton = apr_palloc (pool, sizeof (*baton));
> + svn_stream_t *s = svn_stream_create (baton, pool);
> +
> + /* Make sure EOL_STR and KEYWORDS are allocated in POOL, as
> + required by create_translation_baton() */
> + if (eol_str)
> + eol_str = apr_pstrdup (pool, eol_str);
> + if (keywords)
> + keywords = apr_hash_copy (pool, keywords);
That's only a shallow copy of the hash - is that any good?
> @@ -1256,9 +1469,19 @@
> src_stream = svn_stream_from_aprfile (s, pool);
> dst_stream = svn_stream_from_aprfile (d, pool);
>
> + /* No reason to choose either source or destination,
> + other than to exercise all code paths */
> + if (expand)
> + src_stream = svn_subst_stream_translated (src_stream,
> + eol_str, repair,
> + keywords, expand, pool);
> + else
> + dst_stream = svn_subst_stream_translated (dst_stream,
> + eol_str, repair,
> + keywords, expand, pool);
It's not this function's job to exercise your new code paths! By all means try
this as a form of regression test but, for committing to the repository, if you
can make this function simpler by cutting out the "if" and one of the paths,
please do, although it seems to me that it would be even simpler, and thus
better, if you just left it how it was. Presumably you will soon add some code
that really needs the translated stream, and that will exercise it.
> +
> /* Translate src stream into dst stream. */
> - err = svn_subst_translate_stream3 (src_stream, dst_stream, eol_str,
> - repair, keywords, expand, pool);
> + err = svn_stream_copy (src_stream, dst_stream, pool);
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Add a keyword+eol translating stream
Posted by Erik Huelsmann <eh...@gmail.com>.
On 11/28/05, Branko Čibej <br...@xbc.nu> wrote:
> Erik Huelsmann wrote:
> > lundblad suggested I change blame not to use the extra intermediate
> > file I made it use for certain EOL style settings. In order to do so,
> > I need a diff implementation which runs on streams (I'm working on
> > it!), and the patch below, which translates a stream on-the-fly.
> >
> Ah, finally. :)
>
> Can you please add a TODO to the libsvn_subr/config_file.c
> implementation, so that (someone, somewhen) will replace the STDIO stuff
> in there with this EOL-translating stream?
Sure, but I may just incorporate it into this change to justify it in
its own right: I need to make a streamy diff before i can do what
lundblad suggested I do for blame.
bye,
Erik.
Re: [PATCH] Add a keyword+eol translating stream
Posted by Branko Čibej <br...@xbc.nu>.
Erik Huelsmann wrote:
> lundblad suggested I change blame not to use the extra intermediate
> file I made it use for certain EOL style settings. In order to do so,
> I need a diff implementation which runs on streams (I'm working on
> it!), and the patch below, which translates a stream on-the-fly.
>
Ah, finally. :)
Can you please add a TODO to the libsvn_subr/config_file.c
implementation, so that (someone, somewhen) will replace the STDIO stuff
in there with this EOL-translating stream?
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org