feat: support partial_attr for struct fields

[aschey]

Related to #19

This adds support for the existing derive attribute partial_attr in struct fields. This is useful if you need to derive something on the generated partial struct that requires specifying attributes on struct fields. Being able to derive clap's traits on the partial structs is the main motivation for this, but it also works for any other trait (derive_more is the example I chose for the test).

This is most of the work required to support using partial structs with clap. More info in this comment: #19 (comment)

[LukasKalbertodt]

Thanks for the PR, this is a pretty obvious missing feature in confique and your idea to use it for clap is also good! (Will answer that thread separately).

I just have a bunch of small comments.

[LukasKalbertodt]

I had some thoughts about partial_attr and am interested in your opinion, since you're here anyway :D

I dislike how it looks:

• My brain always reads partial_attr as "an attribute that is partial", i.e. not as the intended "an attribute for the partial type". So every time I read it, it makes me a bit sad.
• It increases nesting, e.g. #[config(partial_attr(derive(Debug)))] -> uhg, ugly! The actual derive(Debug) is now two levels deeper than usual.

So how could one improve this? Using derive(Debug) as example, I could think of the following options:

1. #[forward = "#derive(Debug)" where forward can be any identifier, e.g. config , forward_to_partial , partial
2. #[forward(derive(Debug))] or partial, forward_to_partial, partial_config
3. #[forward(#[derive(Debug)])] or partial, forward_to_partial or partial_config
4. #[config(forward = derive(Debug))] or partial =
5. #[config(@forward derive(Debug))] or @partial
6. #[config(forward: derive(Debug))] or partial:
7. #[config(@derive(Debug))] or other prefixes like !
8. #[config(forward(derive(Debug)))] -> like the current, just change partial_attr to forward (or partial)

With (1) I don't like the need for quotes, as it destroys syntax highlighting for the attribute. For (2) und (3), the derive macro would need to reserve another derive macro helper attribute and sth like forward might clash with other derive macros? (7) is potentially hard to search the docs for? (8) does not improve on the nestedness.

Do you have any thoughts on this? Do you agree with the problems I see with the current solution? Do you think any of the proposed solutions is an improvement? Which is best then? Any other ideas?

[aschey]

I had some thoughts about partial_attr and am interested in your opinion, since you're here anyway :D

Sure, happy to provide some thoughts here. I do agree that the current level of nesting is a bit excessive.

1-3 - I agree that reserving another attribute name wouldn't be ideal. I think most proc macros tend to stick to a single attribute helper (for struct level attributes, at least), so it's easier to remember.

5-7 - introduces some syntax that looks a bit unconventional to me. I think that might be confusing for some users.

8 - I agree doesn't solve the nesting problem, so that's not ideal.

That leaves 4, which I think is a good option. Using = is also consistent with other options in the config attribute like #[config(validate = some_validate_func)]. Personally, I think #[config(partial = derive(Debug))] is a bit clearer than #[config(forward = derive(Debug))] since the meaning of forward isn't entirely evident just from looking at the code, but either way works. Would you like me to make this change? I could do it in this PR or a separate one.

[LukasKalbertodt]

Thanks a bunch and sorry for the delay! Code looks nice. I just removed the last commit (the CI change) since I already fixed that in main. That allowed me to merge.

Regarding the change to partial_attr(...): I need to think about it a bit more and then I'll implement the change myself, but thanks for the offer! And thanks for your thoughts of course!

Regarding the general clap story: as you said, with this PR, a big blocker is solved. I want to dig into the issue myself again soon and figure out what's already possible and if it's maybe already enough. Same goes for the Option problem in the relevant issue. A release of this PR will have to wait for that.

[LukasKalbertodt]

I did release this PR now as 0.3.1 with a bunch of smaller things. It doesn't fully support your use case yet (due to the Option issue), but it's still a useful feature one can publish.

Another aside: I am considering renaming Partial to Layer (including all mentions of "partial"). Among other things, it would also slightly improve the problem here with partial_attr reading as "attributes that are partial": layer_attr to me reads as "attribute of the layer". I might still want to change the syntax a bit but yeah. What do you think about this change?

[aschey]

Thanks for getting this merged!

I am considering renaming Partial to Layer

"Layer" makes sense to me, especially since "layered configuration" is a pretty standard term these days. I've seen some libraries treat partials and layers as two distinct concepts (schematic, for example). Partials and layers seem to have the same conceptual meaning in confique though, so I think it makes sense to combine the terms here.

[LukasKalbertodt]

For the protocol: I renamed "partial" to "layer" now but did not change the syntax of the attribute. None of the options were really all that nice. The rename already solved my main problem (the weird way my brain read it). Only remaining problem is the two-additional layers of nesting. But oh well 🤷