Re: [Monotone-devel] line endings

[Richard Levitte - VMS Whacker]

In message <address@hidden> on Thu, 02 Feb 2006 00:11:51 -0500, Yury Polyanskiy 
<address@hidden> said:

ypolyans> Well I think per-file certificate is something that doesn't
ypolyans> fit in monotone's model (certificates are attached to
ypolyans> revisions not files).

I think they're called "attributes" :-).  Howard did say he is new at
this...

ypolyans> In fact I think current implementation with
ypolyans> get_linesep_conv() includes every other model. IF only I
ypolyans> could check for "merge_manual" attribute from inside the
ypolyans> hook. As nobody answers I assume monotone doesn't have
ypolyans> internal mapping filename->attributes which can be checked
ypolyans> from hook.

I think your guess is correct, and that's a shame.

ypolyans> BTW, I don't understand all the buzz about bad binary files
ypolyans> handling.  Maybe monotone COULD improve heuristics about
ypolyans> detecting binary files but in any case user has complete
ypolyans> control over the process by setting manual_merge attribute.

As it is right now, it does the wrong thing.  When you commit,
read_localized_data() is used to read the contents of each file.  That
function checks the values returned from get_linesep_conv(), and if
they differ, it will have the lines split and then joined using the
first of the two values returned by get_linesep_conv().  As you just
said yourself, there's no way to check if the "manual_merge" attribute
is set for that file (as far as I understand).

ypolyans> Overall I see two problems with line endings by now:
ypolyans> (a) conversion should not touch CR's if I want to convert
ypolyans>     LF's only.

I'm playing with that, even though I disagree with the principle.

ypolyans> (b) it should be possible to check for manual_merge
ypolyans>     attribute from hook

Agreed, big time.  (I only disagree with the attribute name, but
that's a minor issue)

ypolyans> (c) default get_linesep_conv() should return {"LF",
ypolyans>     SYSTEM_SEP } for all files which are not marked by
ypolyans>     manual_merge attr.

I disagree with the return value.  No hook should be able to say what
the internal line ending should be.  In my opinion, it should only
return SYSTEM_SEP.  And for completeness, it should be able to return
"" to say that this file shouldn't be touched.

I must say that I feel nervous handing the decision to a lua hook.
There's the strong possibility that a user somewhere will get
"creative" and that chaos will follow.  If there's an attribute saying
that a file shouldn't be transformed, the internals of monotone should
be able to detect that and avoid doing anything with that file.

ypolyans> That I think would satisfy everyone and does not (except for
ypolyans> b) imply a great change in current code.

OBTW, you mentioned a while ago that files with no line ending at the
end of the file would get one added because of the way
split_into_lines() and join_lines() work.  I've been playing with
that, and it's a lot tougher to change than you might think.  The diff
code depends on this behavior, and possibly the merge code in
diff-patch.cc as well.  I'm currently playing around with the diff
code, and frankly, I cringe at the thought of touching the merge code
for this...

Cheers,
Richard

-----
Please consider sponsoring my work on free software.
See http://www.free.lp.se/sponsoring.html for details.

-- 
Richard Levitte                         address@hidden
                                        http://richard.levitte.org/

"When I became a man I put away childish things, including
 the fear of childishness and the desire to be very grown up."
                                                -- C.S. Lewis