-
-
Notifications
You must be signed in to change notification settings - Fork 264
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
Add to support ///
and //!
syntax for add doc comment for rules.
#765
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great work! sadly, it looks like it's a semver breaking change:
Checking pest_meta
Checking <unknown> v2.5.0 -> v2.5.2 (patch change)
Completed [ 0.141s] 33 checks; 31 passed, 2 failed, 0 unnecessary
--- failure constructible_struct_adds_field: externally-constructible struct adds field ---
Description:
A pub struct constructible with a struct literal has a new pub field. Existing struct literals must be updated to include the new field.
ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
impl: https://github.com/obi1kenobi/cargo-semver-check/tree/v0.15.2/src/queries/constructible_struct_adds_field.ron
Failed in:
field Rule.comments in meta/src/ast.rs:22
field ParserRule.comments in meta/src/parser.rs:55
field OptimizedRule.comments in meta/src/optimizer/mod.rs:105
--- failure enum_variant_added: enum variant added on exhaustive enum ---
Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
impl: https://github.com/obi1kenobi/cargo-semver-check/tree/v0.15.2/src/queries/enum_variant_added.ron
Failed in:
variant Expr:LineDoc in meta/src/ast.rs:93
variant ParserExpr:LineDoc in meta/src/parser.rs:171
variant OptimizedExpr:LineDoc in meta/src/optimizer/mod.rs:140
Final [ 0.161s] semver requires new major version: 2 major and 0 minor checks failed
not sure if it's possible to do in a non-breaking way (e.g. under a feature flag)... maybe mark that enum #[non_exhaustive]
and make that comments
field private (instead of public)?
Looks like we can't do it without the breaking changes. We must add
|
b40da0f
to
02356b1
Compare
} | ||
} | ||
|
||
pub(crate) fn generate( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This because of themod generator
is not public.
So I changed it to pub(crate)
.
e0580cd
to
ed1c818
Compare
@tomtau I have rewrite the implementation without make breaking changes. |
ed1c818
to
34385e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome work if it was made non-semver breaking! I'll have a look at in more detail hopefully soon
@@ -7,11 +7,12 @@ | |||
// option. All files in the project carrying such notice may not be copied, | |||
// modified, or distributed except according to those terms. | |||
|
|||
grammar_rules = _{ SOI ~ grammar_rule+ ~ EOI } | |||
grammar_rules = _{ SOI ~ grammar_doc* ~ (grammar_rule)+ ~ EOI } | |||
|
|||
grammar_rule = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it'll be good to "dogfood" this change in the meta grammar and use it to add doc comments, but that could be done in a follow up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could possibly do that in a follow up PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the non-breaking version seems all right, but if possible, could DocComment
hide its internal fields (for a better "encapsulation") ?
cf4a768
to
90abcb4
Compare
#770 was merged -- could you rebase on top of the latest master? |
Resolve pest-parser#748 For example: ```rust //! A parser for JSON file. /// Matches object, e.g.: `{ "foo": "bar" }` object = { "{" ~ pair ~ ("," ~ pair)* ~ "}" | "{" ~ "}" } ``` should generate: ```rust /// A parser for JSON file. enum Rule { /// Matches object, e.g.: `{ "foo": "bar" }` object, } ```
8bb2e3a
to
8fba851
Compare
8fba851
to
63985fc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, just a small nit about the string construction, but ok to merge
4d9d1af
to
4572232
Compare
63410d2
to
0cd5af4
Compare
0cd5af4
to
279e18d
Compare
Resolve #748
For example:
should generate: