feat: Synopsis to rule stanza, display synopses in dune show#11609
feat: Synopsis to rule stanza, display synopses in dune show#11609maxim092001 wants to merge 61 commits intoocaml:mainfrom
Conversation
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
c4e5fac to
796679e
Compare
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
4883dca to
765d926
Compare
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
| val add_deps | ||
| : t | ||
| -> ?loc:Stdune.Loc.t | ||
| -> ?synopsis:Synopsis.t option |
There was a problem hiding this comment.
I don't get what would the synopsis mean in this instance. Does this mean an alias can have multiple actions attached to it with different descriptions? When do we make use of this?
There was a problem hiding this comment.
It is used in
dune/src/dune_rules/simple_rules.ml
Line 139 in 6f5a145
Does this mean an alias can have multiple actions attached to it with different descriptions?
Yes, it has multiple rules with different targets attached to it.
This is essentially what allows us to print synopses in dune show aliases
dune/src/dune_rules/simple_rules.ml
Line 144 in 6f5a145
Attaches synopsis to targets, so it can be showed in dune show targets
There was a problem hiding this comment.
In dune, there's currently two ways to attach dependencies to an alias. First, using the target of a rule:
(rule
(alias ..))
Second, by attaching it using the alias stanza:
(alias
(name ..)
(deps ..))
In both cases, we're attaching an edge from a target to the alias in that directory. I think it would be wrong to attach an edge to one form but not the other.
But more over, it doesn't even seem right to allow annotating this edge at all. In other words, you're adding the ability to add the same target to different aliases with a different description. Is that really what we're looking for? The target should have one description.
I think it would make more sense to attach the description somewhere in Dune_engine.Rule.t. E.g see the info module for example. For loaded aliases, we don't have an explicit type, but I imagine allowing for one synopsis per alias name per directory is the right way to go.
There was a problem hiding this comment.
In 5ff4fd9 and 4f204c3 I removed passing synopsis through add_deps.
I also removed ability to add synopsis to alias stanza.
Now, you can only add alias to rule stanza and tests are passing as before. You can still get all aggregated synopses for alias if it attached to multiple rules.
I think it would make more sense to attach the description somewhere in Dune_engine.Rule.t.
Yes, I believe it is attached there now
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
…dune into alias-with-synopsis
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
692b097 to
a1fd6ec
Compare
|
@Leonidas-from-XIV I've added docs in a1fd6ec. Is there anything else I need to do from administrative PoV? For changelog entry, do I need to add this feat to Changes.m? |
No, please add a file called |
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
…dune into alias-with-synopsis
|
@Leonidas-from-XIV added changelog file in 5ebd4fc Let me know if there is anything else that needs to be done! |
05e18ab to
95471b3
Compare
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
95471b3 to
4f204c3
Compare
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
Signed-off-by: Maxim Grankin <maximgran@gmail.com>
2434f0a to
5ff4fd9
Compare
|
Failures are the same as on main branch |
Implementation to resolve #11522
This PR makes multiple changes:
(synopsis "Doc")torulestanzalocis notnone) viadune show aliasesdune show targetsAttach synopsis todefaultaliasHad to adjust some tests for new behaviorBumped version to 3.19Merged d274531Some examples from cram tests are: