Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix alignment of strings with preprocessor concatenations #3723

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ArnauBigas
Copy link

Fixes #3086

As described in the related issue, the alignment of strings in a new line after a string which is concatenated by the C/C++ preprocessor is unintuitive in the current version of the code. This PR addresses that by aligning the string in the new line to the start of the string of the previous line, taking into account the possible concatenations, making the indentation/alignment much more intuitive.

@micheleCTDE
Copy link
Collaborator

@guy-maurel
before merging this, I will run a test on my big test repo this weekend.

Copy link
Collaborator

@micheleCTDE micheleCTDE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. please do not use 'prev' directly, use "GetPrev()" to access it.
  2. we should add "negative tests" as well, showing the behavior with the option set an not set.

{
Chunk *tmp = prev;

while ( tmp->prev
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should use "GetPrev()" instead of "prev", which also takes away the need for the first check in the while loop.

@micheleCTDE
Copy link
Collaborator

By the way, welcome to uncrustify, good effort!

@gmaurel
Copy link
Collaborator

gmaurel commented Jul 1, 2022

a fine work.

Chunk *tmp = prev;

while ( tmp->prev
&& (tmp->prev->Is(CT_WORD) || tmp->prev->Is(CT_STRING)))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please also break and indent the while condition as per current style.

@micheleCTDE
Copy link
Collaborator

@guy-maurel before merging this, I will run a test on my big test repo this weekend.

this has been done, no regressions 😄

Copy link
Collaborator

@gmaurel gmaurel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine work, thanks and welcome

@gmaurel
Copy link
Collaborator

gmaurel commented Aug 25, 2022

@ArnauBigas
Please have a look at the change request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

indent_align_string should align to first, not last
3 participants