Skip to content

Conversation

@Yamakaky
Copy link
Contributor

No description provided.

@tailhook
Copy link
Owner

Can you add a test for this feature? I'm not sure what are these imeta parameters at all and how we can apply meta parameters to "methods" which aren't actually the methods.

@Yamakaky Yamakaky changed the title Propagate imeta to method definitions. Propagate variants attributes to method definitions. Nov 19, 2016
@Yamakaky
Copy link
Contributor Author

In fact, the attributes specified to the variants of the error were propagated to the enum definition but not to the various matches in the methods definitions.

@Yamakaky Yamakaky mentioned this pull request Nov 21, 2016
@ticki
Copy link

ticki commented Jan 25, 2017

Can this be merged yet, @tailhook?

@tailhook
Copy link
Owner

Well, sorry for being silent for so much time here.

The problem that might be with this PR, is that cfg(xx) meta attributes might work. But we need to figure out how all other things work, like doc(..) and what else can be on variants?

While it makes sense propagate conditional compilation attributes (i.e. cfg(xx)) to match statement, others should not be propagated there, right? Does rust somehow splits them as meta an imeta or does rust allows to put doc(..) comments on match statements? (sorry, hadn't had much time to test it yet)

@ticki
Copy link

ticki commented Jan 25, 2017

imeta isn't a segment type, but a macro variable name. rustc expands document comments as #[doc = "..."], which works on item segments (e.g. impl, struct, enum, etc. etc.). I'm not sure why you want it to work on match statements? It doesn't seem necessary to me? I might be missing something.

@tailhook
Copy link
Owner

imeta isn't a segment type, but a macro variable name. rustc expands document comments as #[doc = "..."], which works on item segments (e.g. impl, struct, enum, etc. etc.).

Ah, right. I've misread it at a glance.

I'm not sure why you want it to work on match statements? It doesn't seem necessary to me? I might be missing something.

It's there in this pull request:

https://github.com/tailhook/quick-error/pull/30/files#diff-b4aea3e418ccdb71239b96952d9cddb6R529

Because conditional compilation requires it:

https://github.com/tailhook/quick-error/pull/30/files#diff-b4aea3e418ccdb71239b96952d9cddb6R1144

So I guess this will break docstrings on variants (yet to check)

@tailhook tailhook merged commit d13e395 into tailhook:master Apr 24, 2017
@tailhook
Copy link
Owner

From what I observe it works fine. Thanks, for contributing! Sorry for sooooo long delay.

@Yamakaky Yamakaky deleted the patch-1 branch April 24, 2017 19:17
@Yamakaky
Copy link
Contributor Author

^^

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.

3 participants