feat: expose comments to parsing without changing API#178
Conversation
meta-cla
Bot
added
the
CLA Signed
|
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D96525032. (Because this pull request was imported automatically, there will not be any future comments.) |
JakobDegen
left a comment
JakobDegen
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
|
@ndmitchell merged this pull request in 992542c. |
Summary: This should allow me to do things like merge BUILD.in comments for gazelle into the resulting BUILD, or other such things. It doesn't change the API so current usage is unaffected. X-link: facebook/starlark-rust#178 Reviewed By: JakobDegen Differential Revision: D96525032 Pulled By: ndmitchell fbshipit-source-id: e1acb3b06a58580c37128501145dd3c5f1ed45ca
|
Thanks for the diff! I implemented it in a slightly different way which avoids doing the string allocation, since with a Span we actually have all the info we need to get the underlying string stored in the AstModule. I also added some tests, which should be sufficient to confirm it still does what you need. |
|
@ndmitchell Thanks so much for getting this merged in (and yeah, I was going to mention that string alloc in there, so thanks for fixing that up). Any chance we'll be getting a new release on crates.io soon, then, so we don't have to hit our famously reliable friend github to clone the repo for this patch? |
|
@nataliejameson working on the release now. Hoping to get it out today. |
|
#thanks I have no idea if this is still a thing, but thank you! |
|
See #194 for the issue tracking a new release. |
4 participants
This should allow me to do things like merge BUILD.in comments for gazelle into the resulting BUILD, or other such things. It doesn't change the API so current usage is unaffected.