Here are the changes we've discussed in D8478.

I haven't run this myself but the only regression you have mentioned compared with D8478 is that [[D8478#98991 | there is no No newline at end of file warning]] (all the other points are rendering issues). While I do think it is more important to have this fix accepted, it does seem technically correct to avoid losing that non-EOL incidentally. Based on your example XML response from SVN I made a sample script and the presence and lack of EOL are certainly exhibited in the output of SimpleXML. I'm not quite sure where it's getting lost here but somehow was not in D8478.

The property text just is added to em tag an maybe nl2br is applied later. Since each property value is rendered in td the presence of br at the end won't really affect the output. So it's not lost, but in HTML tailing br and no br at the end of td content look the same.

I guess for file content diffs there is a special code that detects such trailing \n absence and prints special line at the end. Although I might not implemented in favor of single-line properties where it would only make extra noise in the output. The svn:ignore property for example can have value in 1 line (just 1 folder/file ignored) and we'll not be showing no new line at the end in that case, but showing it for same property if it has 2 lines in value, then output would be inconsistent.

Sorry, this is all coming back to me more slowly than it should have. For the property, the output from my SVN diff-vs-0 calls were generating that message, not Phabricator.

Property changes on: genini
___________________________________________________________________
Added: svn:executable
## -0,0 +1 ##
+*
\ No newline at end of property

This property was probably added by TortoiseSVN. I do not know off-hand whether other clients also fail to EOL.

So, this code is probably more accurate and the presence or lack of the no-EOL message should be handled when being rendered instead of here.

I guess for file content diffs there is a special code that detects such trailing \n absence and prints special line at the end.

It looks like svn cat does not add the no-EOL message like diff does so I agree that there is likely handling somewhere else for that. Regardless, it would seem that D8478 was innappropriately introducing this message so I think this revision is good to go in my opinion (not that that is worth a whole lot :] ).

I can manually add missing new line message and see how it gots rendered.

Just tried that and it doesn't render the same was as in regular diff (greyed out).

For file:

For properties:

It seems that nice diff engine, that isn't used on property display takes care of displaying line numbers and proper color of no new line at the end messages.

Other than D8478 prefixing a '\', the latter looks the same... aka, wrong. :]

I say leave it as is without adding the no-EOL warning here. This seems to be a rendering issue that should be handled in that code instead.

Then I guess it's ready to be merged someday.

what's the status on this?

In D10977#140535, @eadler wrote:

what's the status on this?

It works on my install. I have no idea, why it isn't merged to upstream.