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: tags when using trailing + regular comments #46

Merged
merged 2 commits into from Aug 16, 2021
Merged

fix: tags when using trailing + regular comments #46

merged 2 commits into from Aug 16, 2021

Conversation

lrstanley
Copy link
Collaborator

This PR fixes an issue with the new trailing comment functionality that I didn't take into consideration when first implementing it.

When you have the following configuration:

message URL {
  // scheme defines what protocol schema should be used (e.g. http, https, etc).
  string scheme = 1; // @inject_tag: valid:"http|https"
  [...]
}

This would cause no tags to be added, because if any comment (even one without tags) was defined in the Doc struct field, it wouldn't try and parse the trailing comments.

This PR does two things:

  • Fixes the above issue, so you can still have a regular comment in the Doc field, and have trailing tags defined.
  • If tags are defined in both locations, the trailing comment takes precedence.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 77

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 71.505%

Totals Coverage Status
Change from base Build 74: 0.0%
Covered Lines: 133
Relevant Lines: 186

💛 - Coveralls

@lrstanley
Copy link
Collaborator Author

@favadi are you ok with me merging changes in myself as long as they're not large, or would you prefer to review?

@favadi
Copy link
Owner

favadi commented Aug 16, 2021

@lrstanley feel free to merge anything yourself, as long as it is backward compatible.

@lrstanley lrstanley merged commit 7b35a50 into favadi:master Aug 16, 2021
@lrstanley lrstanley deleted the bugfix/multiple-comments branch August 16, 2021 19:53
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.

None yet

3 participants