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 parsing token components with parenthesis without spaces #254

Merged
merged 6 commits into from
Sep 30, 2018

Conversation

ilyapuchka
Copy link
Collaborator

Resolves #253

@djbe
Copy link
Contributor

djbe commented Sep 26, 2018

Just being pedantic here, but we want to make sure we cover all cases:

Does this handle a (badly written) expression like:

(one and two )or three

@AliSoftware
Copy link
Collaborator

Good point. If you're asking the question @djbe that means we need a test to cover it 😉

@ilyapuchka
Copy link
Collaborator Author

ilyapuchka commented Sep 26, 2018

it won't handle two )or though it should, good catch

Copy link
Contributor

@djbe djbe left a comment

Choose a reason for hiding this comment

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

We may want to keep an eye on performance with those multiple repeated hasPrefix/hasSuffix tests (don't want to end up having to do another #226).

Don't forget the changelog entry! 😄

} else if word != "(" && word.hasPrefix("(") {
components.append("(")
} else if word != "(" && word.hasPrefix("(") || word != ")" && word.hasPrefix(")") {
components.append(String(word.first!))
Copy link
Contributor

Choose a reason for hiding this comment

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

Please avoid force unwraps, especially in Sources.

You could:

  • use word[0]
  • unroll the if test into a separate one
  • use prefix(1)
  • ...

@ilyapuchka ilyapuchka changed the title fix parsing token components with brackets without spaces Fix parsing token components with brackets without spaces Sep 30, 2018
@ilyapuchka ilyapuchka changed the title Fix parsing token components with brackets without spaces Fix parsing token components with parenthesis without spaces Sep 30, 2018
@ilyapuchka ilyapuchka merged commit 01afae9 into master Sep 30, 2018
@ilyapuchka ilyapuchka deleted the fix-brackets-parsing branch September 30, 2018 20:57
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.

3 participants