Skip to content

Fix multiline comment indentation #4617

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

Conversation

davidBar-On
Copy link
Contributor

Fix an issue in light_rewrite_comment() when formatting 2nd and following lines that start with * in a multiline comment. Currently, the function prefix it with the byte preceding the *, so it will be aligned with the comment opening /*, assuming this character is a space. The problem is that the preceding byte may be a tab or even may not exist when the * starts the line.

This fix makes sure that the * is prefixed by a space character.

@calebcartwright
Copy link
Member

Remind me, this is the scenario when there's a hard tab within the start and end of a a multiline block comment correct?

Also, could you do a rebase on master to fix the i686 windows jobs?

@davidBar-On
Copy link
Contributor Author

Remind me, this is the scenario when there's a hard tab within the start and end of a a multiline block comment correct?

The issue is when hard tab is preceding the * at the second (and following) line of a multiline comment. The previous code used the char preceding the * in the original code to align the * with the /* of the first line. When this char is hard tab indentation was to the next tab place and not just by one space.

could you do a rebase on master to fix the i686 windows jobs?

Done

@calebcartwright
Copy link
Member

could you do a rebase on master to fix the i686 windows jobs?

Done

Hmmm. Something's not looking quite right here (note the commit count and file diffs). Could you please take another look? The associated commits are relatively new so I largely remember what's already been changed, but the jump in the diff size requires a bit more human effort to identify the actual changed bits.

Please also note that with the current commit set in the PR all the original authors have been tagged in and will get spammed with notifications from all updates/posts/etc. so if it's easier to just move the relatively small proposed changes to a new branch and PR that'd be alright in this case.

@davidBar-On
Copy link
Contributor Author

if it's easier to just move the relatively small proposed changes to a new branch and PR that'd be alright in this case.

Resubmitted the changes in PR #4658 and closing this issue.

Please also note that with the current commit set in the PR all the original authors have been tagged in and will get spammed with notifications from all updates/posts/etc.

The issue was cased because I did the rebase by git pull --rebase. I was not aware of the side effects.

@davidBar-On davidBar-On deleted the fix_multiline_comment_indentation branch January 20, 2021 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants