<div dir="ltr"><div class="gmail_extra">I think the problem is that to me this is not a formatting decision (although this might be irrational). To me, it is much closer to a file encoding decision. Also, I don't think this decision is part of any of the coding styles you have mentioned (at least not to a point where somebody has bothered to write it into the style guide).</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">However, I don't disagree with the fundamental concept of clang-format inserting these newlines. What I somewhat doubt is that it is useful as part of the clang-format library. If I write a tool that refactors a certain piece of code (e.g. as done in the unit tests), I do not want clang-format (more precisely libFormat as "clang-format" is the binary) to mess with my line-end/file-end decisions. I can very well use the library to format small bits and pieces of code that aren't even meant to be full files.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">Therefore, I think this particular piece of logic belongs in clang/tools/clang-format/ClangFormat.cpp. Right before saving a file (or printing to stdout), we could ensure that there is a newline at the end (possibly guarded on a style option or command line flag).</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">On Sun, Oct 6, 2013 at 3:44 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 06/10/2013 14:13, Daniel Jasper wrote:<br>
> Actually, the C++11 standard says:<br>
> "A source file that is not empty and that does not end in a new-line<br>
> character, or that ends in a new-line character immediately preceded<br>
> by a backslash character before any such splicing takes place, shall<br>
> be processed as if an additional new-line character were appended to<br>
> the file."<br>
<br>
</div>You're right that it's not a hard error in C++, only C, and the remarks<br>
should be updated to reflect that.<br>
<div class="im"><br>
><br>
> I think this pretty much implies that source files are allowed to not<br>
> end in a newline. Also, I think it is fine for clang-format to not<br>
> alter its input file in this regard. If your VCS requires this,<br>
> configure your editor appropriately ..<br>
<br>
</div>If my editor got formatting right, I wouldn't need clang-format :-)<br>
<br>
Newline at EOF is certainly part of the coding style on the WebKit<br>
project and checked for during code review. It's always a pain to miss<br>
this and have to go back and fix after the fact.<br>
<br>
I can't speak for LLVM but Benjamin Kramer's commit r191857 (Add newline<br>
at eof) suggests it's also part of the coding style here.<br>
<br>
Then there's clang r189110 (Respect -Wnewline-eof even in C++11 mode,<br>
<rdar://problem/12922063>) showing that Apple customers also consider<br>
this important with C++ code.<br>
<br>
I'm guessing from your response that it's not part of the Google coding<br>
style, so I'll make sure that this check is disabled for C++ sources in<br>
GoogleStyle in the next revision of the patch.<br>
<br>
That leaves ChromiumStyle which I don't know enough about. Anyone?<br>
<br>
Thanks for the feedback.<br>
<span class="HOEnZb"><font color="#888888"><br>
Alp.<br>
</font></span><div class="im HOEnZb"><br>
<br>
><br>
><br>
> On Sun, Oct 6, 2013 at 2:10 PM, Alp Toker <<a href="mailto:alp@nuanti.com">alp@nuanti.com</a><br>
</div><div class="HOEnZb"><div class="h5">> <mailto:<a href="mailto:alp@nuanti.com">alp@nuanti.com</a>>> wrote:<br>
><br>
> Hi,<br>
><br>
> I'm just putting this patch out there for now as it's been useful<br>
> to us<br>
> in its current state. Some tweaks are needed before this can land:<br>
><br>
> 1) It needs a lot of existing unit tests to be fixed. The tests<br>
> could be<br>
> fixed before this patch goes in if anyone has the time to volunteer(!)<br>
><br>
> 2) I'm not convinced it's worth even having a 'AllowNoEOL' format flag<br>
> for this. The spec is clear that it's mandatory and many VCS also<br>
> expect<br>
> EOL at EOF to work properly.<br>
><br>
> 3) That means we might do better to add a command line flag to<br>
> identify<br>
> that the code is being formatted for snippets, which uses the<br>
> forSnippets() internally, in which case AllowNoEOL would be made true.<br>
><br>
> 4) There are other rules such as trailing slash in 5.5.1.2 that<br>
> I'm not<br>
> sure are handled yet, though these could be worked on subequent to<br>
> this<br>
> patch landing.<br>
><br>
> Note also that this fixes a bug in clang-format where it was reporting<br>
> an error code for an empty (zero-sized) file. There's no benefit<br>
> to the<br>
> special case so we can just remove that check now.<br>
><br>
> Here's the change:<br>
><br>
> clang-format: Enforce EOL at EOF as required by C standard<br>
><br>
> This enables end-of-line correction as specified for C-family<br>
> languages.<br>
><br>
> C11 5.5.1.2p2 "A source file that is not empty shall end in a<br>
> new-line<br>
> character"<br>
><br>
> The new setting 'AllowNoEOL' and the fluent<br>
> FormatStyle::forSnippets() helper<br>
> are provided for consumers that prefer the old behaviour, which is<br>
> preferable<br>
> for inline code snippets in documentation.<br>
><br>
> Alp.<br>
><br>
> --<br>
> <a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
> the browser experts<br>
><br>
><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>