Add multiple author support to module! #904
Conversation
|
This might also work for #246 |
c714569 to
3cd4350
Compare
rust/macros/module.rs
Outdated
| if let Some(s) = try_string(&mut stream) { | ||
| authors.push(s); | ||
| } | ||
| if try_punct(&mut stream).is_none() { |
There was a problem hiding this comment.
Looks like this would accept any punctuation between the authors. We probably just want ,, right?
rust/macros/module.rs
Outdated
| authors.push(s); | ||
| } | ||
| if try_punct(&mut stream).is_none() { | ||
| assert_eq!(group.delimiter(), Delimiter::Bracket); |
rust/macros/module.rs
Outdated
| assert_eq!(group.delimiter(), Delimiter::Bracket); | ||
| let mut stream = group.stream().into_iter(); | ||
| loop { | ||
| if let Some(s) = try_string(&mut stream) { |
There was a problem hiding this comment.
We can probably expect_string here. Otherwise we'd allow extra commas without any authors in between.
3cd4350 to
dfdb9a3
Compare
|
I suppose we could accept both an array or a single string literal? |
@ojeda do you have comments on this? |
It sounds like a good idea, but then again it introduces complexity that is not really needed, so I am ambivalent. Having always the list, even with 1 entry, is simpler to learn and to implement, and makes it obvious there may be several authors for those that may not know about it. |
0e6bdaf to
29a545e
Compare
Change author into authors, allowing multiple authors of a module. Signed-off-by: wcampbell <wcampbell1995@gmail.com>
29a545e to
0c57fbc
Compare
|
Updated, let me know if this looks good or needs any other modifications. |
|
Sorry, I missed the recent updates - @ojeda is the process now that these should be submitted to the mailing list? |
|
Also the changes now look good to me. |
Either way, I can re-submit the patch. |
Yeah, that is right, we are in the process of submitting things piece by piece to upstream. Please see https://rust-for-linux.com/contributing and https://rust-for-linux.com/branches for some details. |
| @@ -300,6 +300,26 @@ impl ModuleInfo { | |||
|
|
|||
| info | |||
| } | |||
|
|
|||
| /// Parse ["First", "Second"] into Some(vec!["First", "Second"]) | |||
There was a problem hiding this comment.
| /// Parse ["First", "Second"] into Some(vec!["First", "Second"]) | |
| /// Parses `["First", "Second"]` into `Some(vec!["First", "Second"])`. |
i.e. please use Markdown, a period at the end, and the third person.
In addition, this does not add Some(...) -- it is the caller that does it, no?
I would reword it like this instead, if you want to give an example:
| /// Parse ["First", "Second"] into Some(vec!["First", "Second"]) | |
| /// Parses a list of authors. | |
| /// | |
| /// # Examples | |
| /// | |
| /// Parsing `["First", "Second"]` returns a `vec!["First", "Second"]`. |
Change author into authors, allowing multiple authors of a module.
See #244