<div dir="ltr">I think I still can't follow most of your reasoning.<div><br></div><div>There are several different points:</div><div>- Detection of <CR><LF> vs. <LF>: Yes, that is something I hadn't though of, but there are many ways to solve this.</div>
<div>- Adding the newline needs token stream information: I don't understand why.</div><div>- Adding the newline should only be done when the last line is formatted: I disagree. I can see some reason for always doing this (guarded e.g. on a command line flag). Or alternatively, I think only adding the trailing newline if the entire file is formatted (i.e. not formatting ranges are used) should solve most use cases. In practice, people will rarely format just the last line (as it doesn't normally contain anything format-worthy).</div>
<div>- libFormat will have to make the distinction between snippets and full files sooner or later: I am not saying that I am sure that you are wrong, but I also haven't seen a good reason for this yet.</div><div><br>
</div><div>The way I see it currently is:</div><div>- libFormat handles snippets of code. There is no relation to files. It doesn't read files, it doesn't write files, ... Thus, whether a code snippet ends with a newline or not is of no concern to libFormat.</div>
<div>- clang-format (ClangFormat.cpp) uses libFormat to format files. It should be able to fix missing EOF-EOLs.</div><div><br></div><div>I am sorry that you regard the response as negative.. I probably just don't understand some of your reasoning and am thus leaning towards solving this in a slightly different fashion.</div>
<div><br></div><div>Cheers,</div><div>Daniel</div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, Oct 6, 2013 at 6:34 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im"><br>
On 06/10/2013 15:09, Daniel Jasper wrote:<br>
><br>
</div><div class="im">> However, I don't disagree with the fundamental concept of clang-format<br>
> inserting these newlines. What I somewhat doubt is that it is useful<br>
> as part of the clang-format library. If I write a tool that refactors<br>
> a certain piece of code (e.g. as done in the unit tests), I do not<br>
> want clang-format (more precisely libFormat as "clang-format" is the<br>
> binary) to mess with my line-end/file-end decisions. I can very well<br>
> use the library to format small bits and pieces of code that aren't<br>
> even meant to be full files.<br>
<br>
</div>On this point, I think libFormat will have to make the distinction<br>
sooner or later between formatting standalone code snippets and entire<br>
source files. There are various constructs that you want to handle<br>
differently at the beginning or end of a file, with EOL-at-EOF being one<br>
that gets lots of air time in the C/C++ language specifications.<br>
<br>
This is why the patch provides forSnippets(), and libclang's CXComment<br>
has been updated to use it:<br>
<br>
/// \brief Adjust the configuration to format single-line code chunks.<br>
///<br>
/// The updated format will not enforce trailing EOL at EOF and may also<br>
/// apply other adjustments tailored for short inline code snippets<br>
appearing<br>
/// in technical documentation.<br>
FormatStyle forSnippets() const;<br>
<br>
I'm attaching the corresponding unit tests -- all the existing tests<br>
pass fine without modification. It's my fault for not having<br>
communicated this better.<br>
<br>
The trouble with having the feature switched off by default, then<br>
overriding it for complete source files is that it would then be<br>
impossible for the user to switch it off in a configuration file.<br>
<br>
Counterintuitive as it is, I think the most gentle way to add this<br>
feature is to:<br>
<br>
 * Have it on by default.<br>
 * Disable it explicitly for code snippets such as unit tests and<br>
documentation using forSnippets().<br>
 * Permit the user to switch it off in their configuration file if they<br>
really want for some reason.<br>
<br>
This is the only solution I could see that sets us in the right<br>
direction to handle the trailing slash requirements in the C spec (which<br>
are also soft warnings in C++) but maybe there's another way?<br>
<br>
Given the negative response I'll keep this changeset on our internal<br>
branch and maybe we can revisit it for inclusion in Open Source clang in<br>
future once there's more data available.<br>
<br>
Regards,<br>
<div class=""><div class="h5">Alp.<br>
<br>
--<br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</div></div></blockquote></div><br></div></div></div>