D26877: Simplify calls to whitespace() and use it in more places.

2020-02-12 Thread Kevin Ottens 
ervin added a comment.


  In D26877#604672 , @dfaure wrote:
  
  > I don't like it either. It doesn't "read" well.
  >  Looking at cout or qDebug it's much more common to `[the usual stream] << 
[some modifier] << some more stuff`.
  
  
  Yep, that's what made me jump in fact, it feels unusual. :-)
  
  > Maybe it can be solved with naming though.
  >  indentedStream() << ... 
  >  ?
  
  Not bad, it's a good compromise.

REPOSITORY
  R237 KConfig

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

To: tcanabrava, dfaure, ervin
Cc: ervin, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

Re: D26877: Simplify calls to whitespace() and use it in more places.

2020-02-02 Thread Tomaz Canabrava 
I like indentedStream.


On Sun, 2 Feb 2020 at 11:36 David Faure  wrote:

> dfaure requested changes to this revision.
> dfaure added a comment.
> This revision now requires changes to proceed. View Revision
> 
>
> I don't like it either. It doesn't "read" well.
> Looking at cout or qDebug it's much more common to [the usual stream] <<
> [some modifier] << some more stuff.
>
> Maybe it can be solved with naming though.
> indentedStream() << ...
> ?
>
> *REPOSITORY*
> R237 KConfig
>
> *REVISION DETAIL*
> https://phabricator.kde.org/D26877
>
> *To: *tcanabrava, dfaure, ervin
> *Cc: *kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
>

D26877: Simplify calls to whitespace() and use it in more places.

2020-02-02 Thread Tomaz Canabrava 
tcanabrava added a subscriber: ervin.
tcanabrava added a comment.


  I like indentedStream.

REPOSITORY
  R237 KConfig

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

To: tcanabrava, dfaure, ervin
Cc: ervin, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26877: Simplify calls to whitespace() and use it in more places.

2020-02-02 Thread David Faure 
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  I don't like it either. It doesn't "read" well.
  Looking at cout or qDebug it's much more common to `[the usual stream] << 
[some modifier] << some more stuff`.
  
  Maybe it can be solved with naming though.
  indentedStream() << ... 
  ?

REPOSITORY
  R237 KConfig

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

To: tcanabrava, dfaure, ervin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26877: Simplify calls to whitespace() and use it in more places.

2020-01-28 Thread Kevin Ottens 
ervin added a comment.


  I'm not sure I like whitespace() returning the stream and then being used. In 
effect it leads to hiding the stream object making the code more terse indeed 
but less obvious I think.
  Maybe it's a question of personal taste, @dfaure any opinion?

REPOSITORY
  R237 KConfig

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

To: tcanabrava, dfaure, ervin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26877: Simplify calls to whitespace() and use it in more places.

2020-01-24 Thread Tomaz Canabrava 
tcanabrava added reviewers: dfaure, ervin.

REPOSITORY
  R237 KConfig

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

To: tcanabrava, dfaure, ervin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26877: Simplify calls to whitespace() and use it in more places.

2020-01-23 Thread Tomaz Canabrava 
tcanabrava created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
tcanabrava requested review of this revision.

REVISION SUMMARY
  whitespace now returns a textStream so we don't need to stream() << 
whitespace()
  Use whitespace where we had stream() << " followed by 4 spaces
  
  There are plenty of manual space manipulation on the code that I plan to
  get rid of, but that will have to wait as it will break every single
  testcase.

TEST PLAN
  Run unittests

REPOSITORY
  R237 KConfig

BRANCH
  simplify_whitespace

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

AFFECTED FILES
  src/kconfig_compiler/KConfigCodeGeneratorBase.cpp
  src/kconfig_compiler/KConfigCodeGeneratorBase.h
  src/kconfig_compiler/KConfigHeaderGenerator.cpp

To: tcanabrava
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns