[PATCH] [WIP] clang-format: Enforce EOL at EOF as required by C standard

[Daniel Jasper]

I think I still can't follow most of your reasoning.

There are several different points:
- Detection of <CR><LF> vs. <LF>: Yes, that is something I hadn't though
of, but there are many ways to solve this.
- Adding the newline needs token stream information: I don't understand why.
- 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).
- 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.

The way I see it currently is:
- 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.
- clang-format (ClangFormat.cpp) uses libFormat to format files. It should
be able to fix missing EOF-EOLs.

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.

Cheers,
Daniel


On Sun, Oct 6, 2013 at 6:34 PM, Alp Toker <alp at nuanti.com> wrote:

>
> On 06/10/2013 15:09, Daniel Jasper wrote:
> >
> > 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.
>
> On this point, I think libFormat will have to make the distinction
> sooner or later between formatting standalone code snippets and entire
> source files. There are various constructs that you want to handle
> differently at the beginning or end of a file, with EOL-at-EOF being one
> that gets lots of air time in the C/C++ language specifications.
>
> This is why the patch provides forSnippets(), and libclang's CXComment
> has been updated to use it:
>
> /// \brief Adjust the configuration to format single-line code chunks.
> ///
> /// The updated format will not enforce trailing EOL at EOF and may also
> /// apply other adjustments tailored for short inline code snippets
> appearing
> /// in technical documentation.
> FormatStyle forSnippets() const;
>
> I'm attaching the corresponding unit tests -- all the existing tests
> pass fine without modification. It's my fault for not having
> communicated this better.
>
> The trouble with having the feature switched off by default, then
> overriding it for complete source files is that it would then be
> impossible for the user to switch it off in a configuration file.
>
> Counterintuitive as it is, I think the most gentle way to add this
> feature is to:
>
>  * Have it on by default.
>  * Disable it explicitly for code snippets such as unit tests and
> documentation using forSnippets().
>  * Permit the user to switch it off in their configuration file if they
> really want for some reason.
>
> This is the only solution I could see that sets us in the right
> direction to handle the trailing slash requirements in the C spec (which
> are also soft warnings in C++) but maybe there's another way?
>
> Given the negative response I'll keep this changeset on our internal
> branch and maybe we can revisit it for inclusion in Open Source clang in
> future once there's more data available.
>
> Regards,
> Alp.
>
> --
> http://www.nuanti.com
> the browser experts
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131006/8094d9ad/attachment.html>