You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Stenberg <da...@haxx.se> on 2002/01/28 09:14:29 UTC
Re: newline conversion with EDITOR
On Sat, 26 Jan 2002, Branko [UTF-8] Äibej wrote:
> The EDITOR stuff isn't careful about newline conversion in the log
> messages. I edited this log message on Windows with Notepad, and this is
> the result.
Oops.
> May I suggest that we read and write a) files with log messages; b) files
> with prop values; using stdio FILE*s, opend in text mode ("rt", "wt")?
> that'll take care of newline conversion on all systems.
Isn't this just a case where we (I) use bad flags to apr_file_open() ?
Sicne the -F stuff works, and it opens the files with 'APR_OS_DEFAULT' then
wouldn't that be the quick solution to this problem?
--
Daniel Stenberg - http://daniel.haxx.se - +46-705-44 31 77
ech`echo xiun|tr nu oc|sed 'sx\([sx]\)\([xoi]\)xo un\2\1 is xg'`ol
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: newline conversion with EDITOR
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Greg Hudson <gh...@MIT.EDU> writes:
> On Mon, 2002-01-28 at 04:57, Greg Stein wrote:
> > But I don't think we shold use FILE*. Using apr_file_t is portable. If it
> > has drawbacks, then we should fix APR, not workaround it.
>
> I'm a little confused by this claim. FILE * is extremely portable; not
> only is it in ANSI C, but it's also in K&R C. There have been some
> historical stdio bugs, but none I know of which would actually affect
> Subversion.
Well, note that Greg Stein didn't actually claim that FILE* is not
portable, he merely claimed that apr_file_t *is* portable. :-)
I think if only for code consistency it's worth using apr_file_t. I
believe it does offer some features beyond what FILE* offers, such as
finding out the path from the apr_file_t *. (Not that we need this
right now in the $EDITOR case.)
> (Likewise, system() is part of ANSI C, so is also quite portable, but is
> worth ignoring for other reasons.)
The presence of the "system()" call is dependable. Its behavior
w.r.t. any given argument is not.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: newline conversion with EDITOR
Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jan 28, 2002 at 09:36:19AM -0500, Greg Hudson wrote:
> On Mon, 2002-01-28 at 04:57, Greg Stein wrote:
> > But I don't think we shold use FILE*. Using apr_file_t is portable. If it
> > has drawbacks, then we should fix APR, not workaround it.
>
> I'm a little confused by this claim. FILE * is extremely portable; not
> only is it in ANSI C, but it's also in K&R C. There have been some
> historical stdio bugs, but none I know of which would actually affect
> Subversion.
All right, let me rephrase and say that "apr_file_t is a better mapping to
native OS file facilities, yet remains portable." We get to use native file
operations on Win32, BeOS, OS/2, and Netware, rather than their covers for
FILE*.
As Karl says, for consistency, we ought to continue using apr_file_t, and to
add any missing functionality.
> (Likewise, system() is part of ANSI C, so is also quite portable, but is
> worth ignoring for other reasons.)
system() isn't very portable to things like Windows. apr_proc_create()
handles process creation a lot better...
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: newline conversion with EDITOR
Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2002-01-28 at 04:57, Greg Stein wrote:
> But I don't think we shold use FILE*. Using apr_file_t is portable. If it
> has drawbacks, then we should fix APR, not workaround it.
I'm a little confused by this claim. FILE * is extremely portable; not
only is it in ANSI C, but it's also in K&R C. There have been some
historical stdio bugs, but none I know of which would actually affect
Subversion.
(Likewise, system() is part of ANSI C, so is also quite portable, but is
worth ignoring for other reasons.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: newline conversion with EDITOR
Posted by Branko Čibej <br...@xbc.nu>.
Greg Stein wrote:
>I'm a little unclear on where Branko wants the newline stuff to go here.
>IMO, there shouldn't be any newline conversion. The editor should be popped
>up, and we just suck out the data. I guess we kind of need to figure out
>what the newline style is, so that we can properly eliminate lines that
>start with SVN:.
>
>But I don't think we shold use FILE*. Using apr_file_t is portable. If it
>has drawbacks, then we should fix APR, not workaround it.
>
I think I summed that up pretty well in the comment in config_file.c:
/* "Why," you ask yourself, "is he using stdio FILE's instead of
apr_file_t's?" The answer is simple: newline translation. For
all that it has an APR_BINARY flag, APR doesn't do newline
translation in files. The only portable way I know to get
translated text files is to use the standard stdio library. */
(I might have added here that stdio streams are buffered on the C source
level (getc is a macro), while APR files aren't.)
Sometimes we create or read files on the client that have nothing at all
to do with the repository. One such case is the $EDITOR feature, another
is "svn ci -F". For $EDITOR, we have to both create and read the file
with the correct (local) line endings. The stdio library already knows
how to do that. APR does not.
We can either use stdio for accessing these files, fix APR to accept an
APR_TRANSLATED flag at file open, or create a subversion stream adapter
that grafts newline conversion on top of an ordinary APR file (using the
newline conversion code we already have). Personally I don't mind using
stdio for local text files, but I can live with any of these three
alternatives.
(When I wrote the config file parser, using stdio was the quickest
solution, so I did.)
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: newline conversion with EDITOR
Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jan 28, 2002 at 10:14:29AM +0100, Daniel Stenberg wrote:
> On Sat, 26 Jan 2002, Branko [UTF-8] Äibej wrote:
>
> > The EDITOR stuff isn't careful about newline conversion in the log
> > messages. I edited this log message on Windows with Notepad, and this is
> > the result.
>
> Oops.
>
> > May I suggest that we read and write a) files with log messages; b) files
> > with prop values; using stdio FILE*s, opend in text mode ("rt", "wt")?
> > that'll take care of newline conversion on all systems.
>
> Isn't this just a case where we (I) use bad flags to apr_file_open() ?
>
> Sicne the -F stuff works, and it opens the files with 'APR_OS_DEFAULT' then
> wouldn't that be the quick solution to this problem?
Um...
I'm a little unclear on where Branko wants the newline stuff to go here.
IMO, there shouldn't be any newline conversion. The editor should be popped
up, and we just suck out the data. I guess we kind of need to figure out
what the newline style is, so that we can properly eliminate lines that
start with SVN:.
But I don't think we shold use FILE*. Using apr_file_t is portable. If it
has drawbacks, then we should fix APR, not workaround it.
So... I'd say the intended behavior is:
*) get the file as specified by -F or as the output of EDITOR.
*) figure out the newline style (\r, \n, \r\n), and break up the files into
lines. eliminate lines that start with SVN:
*) eliminate leading and trailing blank lines (new behavior!)
*) recompose the log message using Unix newlines: \n
*) send the contents as the log message
We had said that the property stuff should treat all properties as binary.
It is possible that we shouldn't do any newline handling for property files.
Just take the output as is and shove it into the property.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org