Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

Fix comment generation when using other kind of linebreaks#46

Merged
liamnichols merged 1 commit intomainCreateAPI/CreateAPI:mainfrom
ln/fix-multiline-commentsCreateAPI/CreateAPI:ln/fix-multiline-commentsCopy head branch name to clipboard
Jul 28, 2022
Merged

Fix comment generation when using other kind of linebreaks#46
liamnichols merged 1 commit intomainCreateAPI/CreateAPI:mainfrom
ln/fix-multiline-commentsCreateAPI/CreateAPI:ln/fix-multiline-commentsCopy head branch name to clipboard

Conversation

@liamnichols
Copy link
Member

Background

It was reported that when a string that is used in comments uses \r for line breaks, the generated code will incorrectly fail to prefix the line with /// resulting in non-compiling source files.

Fix

From what I could tell, the easiest way to fix this is to separate into components based on anything within the predefined .newlines CharacterSet.

It was a bit tricky to do this using firstIndex(of:) in the existing while loop, whereas it was much easier to use components(separatedBy:). However, I am wary because of this comment:

// Unlike `components(separatedBy: "\n")`, it keeps empty lines.

However when I compared the implementation, I didn't see a difference in output:

Screenshot 2022-06-29 at 10 21 50

@kean, please could you confirm if I missed anything here? It looks like it works as expected using the updated approach but I might have misunderstood 🙏

@liamnichols liamnichols requested a review from kean June 29, 2022 09:42
@liamnichols liamnichols self-assigned this Jun 29, 2022
@liamnichols liamnichols merged commit 4a0154d into main Jul 28, 2022
@liamnichols liamnichols deleted the ln/fix-multiline-comments branch July 28, 2022 08:13
@kean
Copy link
Member

kean commented Jul 30, 2022

I missed the mention. Yes, I just tested it and it looks like it does keep the empty lines. I'm not sure I done it this way originally. The new approach is much better.

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.

Support escape character \r in comments

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.