[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] truncate EDITOR_EOF_PREFIX with previous EOL

From: Olleg Samoylov <olleg_at_mipt.ru>
Date: 2005-07-20 11:06:09 CEST

kfogel@collab.net wrote:

> * Problem (1) is that some text editors always add a trailing eol.
> If you don't want the trailing eol, the solution is not to change
> Subversion, but to use a different editor (or configure your editor
> differently, or write a script to fix up the message in a file
> afterwards, or whatever.) I'm using "eol" to mean "whatever sequence
> of characters represents end-of-line on the client system", by the
> way.

Yes, some editors place EOF only after EOL. IMHO most editors do this.
May be except hex editors. :-)

> * Problem (2) is that the Subversion command-line client
> unconditionally adds a trailing eol when it *prints* log messages, to
> make sure that multiple logs in sequence look pretty. Changing this
> behavior is possible, but would need to be done carefully: we can't
> just stop adding the eol, instead we'd have to refrain from adding it
> when there is already at least one eol present anyway. Your patch
> doesn't do this.

It's not true:
12:24:20 olleg@archie ~/src/test
$ svn log -r HEAD
r6 | olleg | 2005-07-18 22:59:37 +0400 (Mon, 18 Jul 2005) | 1 line

12:24:22 olleg@archie ~/src/test

Here is not trailing eol. Only blank line between revision and log
message. You can test this by command
svn ci -m test

> * Problem (3) is that when you write a log message by allowing
> Subversion to invoke an interactive editor, Subversion places a list
> of changed files at the end of the buffer, and promises to
> automatically remove that list. However, when it removes the list,
> it still leaves the eol that preceded the list.

Yes, it's look like bug. When I write one line message on commit in
editor, message created with two line.

> Since having a trailing eol on a log message is normally considered
> desirable, I think this is a good default behavior for Subversion,

Some like output "svn log" with trailing blank line, some without.
Output of "svn log" must be convenient or customizable, but it's not
matter of this patch.
When I write one line log message, I expect one line log message. I expect:
$ svn log -r HEAD
r6 | olleg | 2005-07-18 22:59:37 +0400 (Mon, 18 Jul 2005) | 1 line

How many blank line may add "svn log" or other subversion client is
other question.

> especially since it's easy to work around -- just remove the change
> list using your editor, making sure to remove the preceding eol as
> well (this assumes your editor doesn't have Problem (1), of course).

Another workaround is use "svn commit --message" and don't use buggy
support of editor. :-)
If a bug have a workaround it don't begin looked like a customizable
feature. :-)

> Based on where you made the patch, I guess you are trying to solve
> problem (3) only.

Fix bug "problem(3)" and try workaround problem(1).

> The actual behavior change is a bit more sophisticated than what you
> wrote, however. Saying that you are removing "the trailing eol" is
> unclear, since at the time the input goes into the function, that eol
> is not trailing (it is somewhere in the middle of the string). Say
> instead "Avoid leaving a trailing eol on the result.", or something
> like that.

English is not my native language and I speak as I can. Anyone free to
change my log message to something better.

> There was a formatting problem, I think: the "+" right after the
> "else" should have begun a new line, right?

Yes, thanks. Strange, I simple copied from xterm to thunderbird. May be

> Also, this algorithm is not sophisticated enough. What if you
> encounter '\r' or '\n' as independent characters in a log message,
> not as part of an eol sequence (on Windows at least, '\n' can stand
> alone outside an eol sequence; on Unix it's always eol, of course).
> Of course, the current code has the same bug, which is not your fault
> at all.

Yes, I take exactly same code to detect EOL from current source.
But may be better use apr_isspace(char)? And remove not only EOL but
trailing space also?

> Finally, you changed the behavior of truncate_buffer_at_prefix(), but
> you did not change its doc string. Don't make the doc string lie
> :-).

Yep, my fault. When I get clear answer about apr_isspace(char) I
resubmit patch.

Olleg Samoylov

Received on Wed Jul 20 23:06:40 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.