D24826: Enforce 100 chars line width

[Roman Gilg]

romangg abandoned this revision.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24826

To: romangg, #frameworks, cullmann
Cc: carewolf, winterz, zzag, kde-frameworks-devel, kde-buildsystem, LeGast00n, 
cblack, bencreasy, michaelh, ngraham, bruns

D24826: Enforce 100 chars line width

[Roman Gilg]

romangg added a comment.


  FYI I'm using a strict 100 chars limit nowadays on all my KWinFT projects 
(with the exception of KWinFT itself for now) and I'm very happy with this 
decision.
  
  As an example you can compare these two otherwise still pretty similar files:
  https://invent.kde.org/plasma/libkscreen/-/blob/master/src/backendmanager.cpp
  https://gitlab.com/kwinft/disman/-/blob/master/lib/backendmanager.cpp
  
  I don't even think about any of this anymore, especially I don't care if 
clang-format moves a function name or puts a bracket here or there. I just 
apply clang-format to the file and that's it. The consistency trumps anything 
else.
  
  Anyway closing this diff as requested.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24826

To: romangg, #frameworks, cullmann
Cc: carewolf, winterz, zzag, kde-frameworks-devel, kde-buildsystem, LeGast00n, 
cblack, bencreasy, michaelh, ngraham, bruns

D24826: Enforce 100 chars line width

[Allan Sandfeld Jensen]

carewolf added a comment.


  I agree. A limit of 100 is arbitrary and harmful. Unfortunately clang-format 
doesn't seem to have a way to make lines over a certain length less desirably, 
but still allowed if the break would be ugly, so it is better to just allow 
long lines.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24826

To: romangg, #frameworks, cullmann
Cc: carewolf, winterz, zzag, kde-frameworks-devel, kde-buildsystem, LeGast00n, 
cblack, bencreasy, michaelh, ngraham, bruns

D24826: Enforce 100 chars line width

[Christoph Cullmann]

cullmann added a comment.


  As said above, I think 100 is a bad idea. Can we close this?

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24826

To: romangg, #frameworks, cullmann
Cc: winterz, zzag, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
bencreasy, michaelh, ngraham, bruns

D24826: Enforce 100 chars line width

[Christoph Cullmann]

cullmann added a comment.


  See the other task: columnlimit 0 leads to endless long lines and is a 
unusable default.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24826

To: romangg, #frameworks, cullmann
Cc: winterz, zzag, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

D24826: Enforce 100 chars line width

[Allen Winter]

winterz added a comment.


  I am a long-time advocate of columnLimits; however, in our modern world of 
programming I think 100 is too short.
  
  240 may be a bit too long: in my personal coding style scripts I try to limit 
to 120.   even 120 is hard to achieve sometimes.
  I feel that 240 is a fair compromise.
  
  -1 for this patch

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24826

To: romangg, #frameworks, cullmann
Cc: winterz, zzag, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

D24826: Enforce 100 chars line width

[Vlad Zahorodnii]

zzag added a comment.


  I suggest to set ColumnLimit to 0 by default and allow projects to override 
it.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24826

To: romangg, #frameworks, cullmann
Cc: zzag, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D24826: Enforce 100 chars line width

[Christoph Cullmann]

cullmann added a comment.


  Perhaps easiest way to see what happens: apply it to one of your things and 
vary the value.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24826

To: romangg, #frameworks, cullmann
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D24826: Enforce 100 chars line width

[Roman Gilg]

romangg added a comment.


  In D24826#551280 , @cullmann wrote:
  
  > As explained in the thread on https://phabricator.kde.org/T11214, this will 
make the formatting ugly as hell, as if you have long method names, stuff is 
broken in arbitrary bad ways.
  >  I don't want to change that, if nobody can avoid the resulting breakage.
  
  
  Does it brutalize the line breaks always or only when the code line does not 
comply with the 100 chars limit? Or from the other side does `ColumnLimit: 240` 
mean that all "unnecessary" manual line breaks will be removed such that there 
are >200 chars lines now where before a document was only with <200? This would 
be even worse.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24826

To: romangg, #frameworks, cullmann
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D24826: Enforce 100 chars line width

[Christoph Cullmann]

cullmann added a comment.


  As explained in the thread on https://phabricator.kde.org/T11214, this will 
make the formatting ugly as hell, as if you have long method names, stuff is 
broken in arbitrary bad ways.
  I don't want to change that, if nobody can avoid the resulting breakage.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24826

To: romangg, #frameworks, cullmann
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D24826: Enforce 100 chars line width

[Roman Gilg]

romangg created this revision.
romangg added reviewers: Frameworks, cullmann.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
romangg requested review of this revision.

REVISION SUMMARY
  The KDE Frameworks style recommended a 100 chars line limit but did not 
enforce
  it. Now with the clang-format file currently a somewhat arbitrary limit of 240
  chars is enforced.
  
  Instead enforce the recommendation of 100 chars per line.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  columnLimit

REVISION DETAIL
  https://phabricator.kde.org/D24826

AFFECTED FILES
  kde-modules/clang-format.cmake

To: romangg, #frameworks, cullmann
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns