clang-format leading whitespace

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

clang-format leading whitespace

Alex Denisov via cfe-dev
I am interested in contributing to clang-format, to add an option to preserve and correct leading indentation.

Right now if you write code such as:

{
....int x = 10;
....
....return x;
}

The line in the middle will be truncated and there is no option to control it.

Some code prefers to have indented leading whitespace, indented to the same level as the previous line.

Would such a contribution be accepted and is anyone willing to help me through the process where necessary?

Thanks
Samuel

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clang-format leading whitespace

Alex Denisov via cfe-dev
On Sat, Nov 11, 2017 at 7:20 AM Samuel Williams via cfe-dev <[hidden email]> wrote:
I am interested in contributing to clang-format, to add an option to preserve and correct leading indentation.

Right now if you write code such as:

{
....int x = 10;
....
....return x;
}

The line in the middle will be truncated and there is no option to control it.

Some code prefers to have indented leading whitespace, indented to the same level as the previous line.

Would such a contribution be accepted and is anyone willing to help me through the process where necessary?

The current reasoning is that with an automated formatter the indented leading whitespace doesn't help any more.
 

Thanks
Samuel
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clang-format leading whitespace

Alex Denisov via cfe-dev
Generally, see here:

I vaguely remember this question being brought up before and I think we decided against it.

On Mon, Nov 13, 2017 at 2:14 PM, Manuel Klimek via cfe-dev <[hidden email]> wrote:
On Sat, Nov 11, 2017 at 7:20 AM Samuel Williams via cfe-dev <[hidden email]> wrote:
I am interested in contributing to clang-format, to add an option to preserve and correct leading indentation.

Right now if you write code such as:

{
....int x = 10;
....
....return x;
}

The line in the middle will be truncated and there is no option to control it.

Some code prefers to have indented leading whitespace, indented to the same level as the previous line.

Would such a contribution be accepted and is anyone willing to help me through the process where necessary?

The current reasoning is that with an automated formatter the indented leading whitespace doesn't help any more.
 

Thanks
Samuel
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clang-format leading whitespace

Alex Denisov via cfe-dev
In reply to this post by Alex Denisov via cfe-dev
Thanks for your feedback, and I appreciate the link and the logic behind it.

Unfortunately, by stripping all trailing whitespace, clang-format has made a policy decision, but with no way to control it via the configuration. I understand it's not desirable for every option to be configurable, but this is something which significantly affects existing code. I propose that it would be good to have an option "TrailingWhitespace: Truncate|Preserve|Fix". Truncate would be existing behaviour. Preserve would not alter leading whitespace on blank lines or touch trailing whitespace. Fix would fix leading whitespace on blank lines, fix trailing whitespace on code, but avoid making any changes in comments except if it was reflowed. It could be more granular but that's where I'd start.

## Some objective problems:

- Existing code which is indented this way would require a lot of changes.
- Markdown (which is useful in comments) is sensitive to trailing whitespace.

It's possible to add a post process script to normalise leading whitespace but not trailing whitespace (e.g. in the case of markdown).

## Some subjective problems:

- It's my preference when I start editing on a line, the indentation is correct already.
- When moving cursor up and down it's my preference that it doesn't jump back and forward when crossing blank lines.

Some of these issues can be fixed by editor configuration/plugins, but it's still not 100% correct.

Thanks for your time.

Kind regards,
Samuel


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clang-format leading whitespace

Alex Denisov via cfe-dev
In reply to this post by Alex Denisov via cfe-dev
As an extra data point I want to tell that even with an automated formatter the indented leading whitespace does help. If you re-indent a block of code and leading whitespace is preserved, then you get a single chunk in the blame of the file. If you eat the leading whitespace then you have a million little pieces in the blame and it’s less clear just what happened. Something like

1111bc501498 (John Doe      2015-05-12 09:23:57 +0000  178) if (something) {
1111bc501498 (John Doe      2015-05-12 09:23:57 +0000  179)   // do something
2222c3ae83d8 (Adam Smith    2014-12-10 19:00:42 +0000  180)
1111bc501498 (John Doe      2015-05-12 09:23:57 +0000  181)   // do some more stuff
1111bc501498 (John Doe      2015-05-12 09:23:57 +0000  182) }

If leading whitespace was preserved after reindentation you would get a single blame chunk. The same applies to diff. Empty lines can make chunks too granular.

As for requirement
be used in a project of significant size

I want to point out that preserving leading whitespace on whitespace-only lines is Xcode default setting. And as the result many projects can end up using this style. For example, a quick check of the top 5 Objective-C repositories with the most stars on GitHub shows that 3 out of 5 are using predominantly this style (rs/SDWebImage, BradLarson/GPUImage, SnapKit/Masonry). It isn’t representative in any way, I just want to show that such style isn’t that obscure.

Thanks,
Volodymyr

On Nov 13, 2017, at 05:19, Daniel Jasper via cfe-dev <[hidden email]> wrote:

Generally, see here:

I vaguely remember this question being brought up before and I think we decided against it.

On Mon, Nov 13, 2017 at 2:14 PM, Manuel Klimek via cfe-dev <[hidden email]> wrote:
On Sat, Nov 11, 2017 at 7:20 AM Samuel Williams via cfe-dev <[hidden email]> wrote:
I am interested in contributing to clang-format, to add an option to preserve and correct leading indentation.

Right now if you write code such as:

{
....int x = 10;
....
....return x;
}

The line in the middle will be truncated and there is no option to control it.

Some code prefers to have indented leading whitespace, indented to the same level as the previous line.

Would such a contribution be accepted and is anyone willing to help me through the process where necessary?

The current reasoning is that with an automated formatter the indented leading whitespace doesn't help any more.
 

Thanks
Samuel
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clang-format leading whitespace

Alex Denisov via cfe-dev


On Sat, Dec 9, 2017 at 2:58 AM, Volodymyr Sapsai <[hidden email]> wrote:
As an extra data point I want to tell that even with an automated formatter the indented leading whitespace does help. If you re-indent a block of code and leading whitespace is preserved, then you get a single chunk in the blame of the file. If you eat the leading whitespace then you have a million little pieces in the blame and it’s less clear just what happened. Something like

1111bc501498 (John Doe      2015-05-12 09:23:57 +0000  178) if (something) {
1111bc501498 (John Doe      2015-05-12 09:23:57 +0000  179)   // do something
2222c3ae83d8 (Adam Smith    2014-12-10 19:00:42 +0000  180)
1111bc501498 (John Doe      2015-05-12 09:23:57 +0000  181)   // do some more stuff
1111bc501498 (John Doe      2015-05-12 09:23:57 +0000  182) }

If leading whitespace was preserved after reindentation you would get a single blame chunk. The same applies to diff. Empty lines can make chunks too granular.

This seems like a very specific and actually quite rare case. And I'd personally be happy to see some hint that John Doe wasn't actually to blame for all of those lines (so that I can remember to run "git blame" with "-w").

As for requirement
be used in a project of significant size

I want to point out that preserving leading whitespace on whitespace-only lines is Xcode default setting. And as the result many projects can end up using this style. For example, a quick check of the top 5 Objective-C repositories with the most stars on GitHub shows that 3 out of 5 are using predominantly this style (rs/SDWebImage, BradLarson/GPUImage, SnapKit/Masonry). It isn’t representative in any way, I just want to show that such style isn’t that obscure.

It totally makes sense for an automated indenter in an editor to add these leading spaces, but I think the use case is different from a formatter. And it makes sense for some projects coming from Xcode to not bother to modify anything afterwards. It doesn't mean that this is the style they *want*.

Thanks,
Volodymyr

On Nov 13, 2017, at 05:19, Daniel Jasper via cfe-dev <[hidden email]> wrote:

Generally, see here:

I vaguely remember this question being brought up before and I think we decided against it.

On Mon, Nov 13, 2017 at 2:14 PM, Manuel Klimek via cfe-dev <[hidden email]> wrote:
On Sat, Nov 11, 2017 at 7:20 AM Samuel Williams via cfe-dev <[hidden email]> wrote:
I am interested in contributing to clang-format, to add an option to preserve and correct leading indentation.

Right now if you write code such as:

{
....int x = 10;
....
....return x;
}

The line in the middle will be truncated and there is no option to control it.

Some code prefers to have indented leading whitespace, indented to the same level as the previous line.

Would such a contribution be accepted and is anyone willing to help me through the process where necessary?

The current reasoning is that with an automated formatter the indented leading whitespace doesn't help any more.
 

Thanks
Samuel
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clang-format leading whitespace

Alex Denisov via cfe-dev
In reply to this post by Alex Denisov via cfe-dev


On Tue, Nov 14, 2017 at 1:42 AM, Samuel Williams via cfe-dev <[hidden email]> wrote:
Thanks for your feedback, and I appreciate the link and the logic behind it.

Unfortunately, by stripping all trailing whitespace, clang-format has made a policy decision, but with no way to control it via the configuration. I understand it's not desirable for every option to be configurable, but this is something which significantly affects existing code. I propose that it would be good to have an option "TrailingWhitespace: Truncate|Preserve|Fix". Truncate would be existing behaviour. Preserve would not alter leading whitespace on blank lines or touch trailing whitespace. Fix would fix leading whitespace on blank lines, fix trailing whitespace on code, but avoid making any changes in comments except if it was reflowed. It could be more granular but that's where I'd start.

## Some objective problems:

- Existing code which is indented this way would require a lot of changes.

That's true for any project starting to use clang-format. That's why we'd suggest to use editor and version control system integrations and only format the lines that are touched.
 
- Markdown (which is useful in comments) is sensitive to trailing whitespace.

I haven't seen a project using markdown in comments and relying on the trailing whitespace. However, even if some do, I think it'd be better for clang-format to recognize this and not remove spaces for those comments specifically.

It's possible to add a post process script to normalise leading whitespace but not trailing whitespace (e.g. in the case of markdown).

## Some subjective problems:

- It's my preference when I start editing on a line, the indentation is correct already.

How often do you even start editing in a currently empty line? Don't you normally start with some existing code and have to insert a line break? clang-format wouldn't help there..

(It's also a really easy habit to loose once you have gotten yourself to rely on clang-format working in your editor).
 
- When moving cursor up and down it's my preference that it doesn't jump back and forward when crossing blank lines.

But the proposal wouldn't really change much here, unless you happen to have the cursor right at the start of the line and it would all be indented the exact same level. Isn't that a really rare case?

Some of these issues can be fixed by editor configuration/plugins, but it's still not 100% correct.

Thanks for your time.

Kind regards,
Samuel


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev