macro_rules_attribute-rs icon indicating copy to clipboard operation
macro_rules_attribute-rs copied to clipboard

adds support for specifying re-export visibility

Open colstrom opened this issue 1 year ago • 4 comments
trafficstars

Thanks for your time and effort on this crate! Your contributions to the Rust ecosystem are valuable and appreciated. ❤️

This resolves #16 and #22.

All credit for the code goes to @tlowerison, all I'm doing here is proposing that their changes be merged upstream (and bumping the version accordingly), since I'm actively using this version. 😁

If there's anything you'd like me to change, feel free to let me know!

Thanks for considering these changes.

colstrom avatar Sep 27 '24 16:09 colstrom

Excellent, thank you. I'm working my way through the suggested changes. My intuition around macros isn't as robust as I'd like it to be, so feedback like this helps me better understand how others think about these things. ^_^

colstrom avatar Sep 27 '24 23:09 colstrom

(forgot to circle back to this, I'll try to do so within the week; do ping me if I don't!)

danielhenrymantilla avatar Sep 30 '24 23:09 danielhenrymantilla

(forgot to circle back to this, I'll try to do so within the week; do ping me if I don't!)

Sounds good. I appreciate that this -- like many (or even most?) OSS projects -- is a volunteer effort, and that maintainers have other responsibilities and personal lives and stuff. What time you can spare, when you can, is appreciated.

No stress on timeline or anything, rust/cargo makes it very easy to depend on forks and PRs and such.

The end goal is to just make sure that useful changes make their way upstream, eventually. ^_^

colstrom avatar Oct 01 '24 16:10 colstrom

Apologies for the large diff in the last change, it's easier to read with git diff --ignore-all-space (or with "Hide Whitespace" option selected in the Diff View options if reviewing through the GitHub UI).

Hopefully this makes code review(s) more comfortable, since each level of nesting is explicit, and the indentation is normalized to 2 spaces (since the macros are quite nested), making it easier to scan and match braces visually, given that some of the suggested changes are directly relating to that stuff.

Reading through the macros after the change, it's much easier for me to spot the differences between them now, and it feels like maybe there would be a way for one to just... call the other? Initially I thought replacing pub #[apply(... with $vis:vis #[apply(... in the one matcher and then having the other (the one without pub) call it with pub(in crate), but apparently that's not allowed because vis fragments can only be followed by ident or ty, and not #. 🤔

Even if that worked, there's still the difference of the #[macro_export] in the one transcriber but not the other, and I don't (yet!) understand how to omit that in a declarative macro, assuming it's possible to do.

colstrom avatar Oct 03 '24 22:10 colstrom

hey y'all, is this still intended to be merged?

Diegovsky avatar Apr 10 '25 17:04 Diegovsky