Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Class modifiers] dartdoc implementation #3329

Closed
itsjustkevin opened this issue Feb 7, 2023 · 7 comments
Closed

[Class modifiers] dartdoc implementation #3329

itsjustkevin opened this issue Feb 7, 2023 · 7 comments
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug

Comments

@itsjustkevin
Copy link

No description provided.

@srawlins srawlins added type-enhancement A request for a change that isn't a bug P1 A high priority bug; for example, a single project is unusable or has many test failures labels Feb 7, 2023
@srawlins
Copy link
Member

srawlins commented Feb 7, 2023

There is real work to do here. If you take this table, we will basically have a getter on each ClassElement for each of the columns aside from Declaration, and will have to do the calculation to guess at what the Declaration was. Or perhaps ClassElement can have getters that do the logic like bool get isBase => isExtendableIn(myLibrary) && !isImplementableIn(myLibrary).

CC @kallentu @bwilkerson for any thoughts.

Or, if we're saying that developers don't want to know if a class is modified with base, but rather that they want to know if a class is extendable in/outside of it's library, maybe we present that.

@kallentu
Copy link
Member

@srawlins What work is needed for this? I'm not sure what we're presenting.

@srawlins
Copy link
Member

@jcollins-g may be planning to work on this.

I'm not sure what exactly we should present, or how. But one example is that we show the word 'abstract' in the breadcrumbs for an abstract class like List.

@jcollins-g
Copy link
Contributor

@kallentu #3354 introduced some basic support that simply translates across the declaration. To complete this support we'll need to add handling for the inferred modifiers once this is added to the analyzer and published, so we're blocked on that bit. (We could of course do that here, too, but I think it's a bad idea to duplicate that functionality.)

@leafpetersen
Copy link
Member

I said this in comments elsewhere, but to reiterate here. The core design principle of this feature is that users never need to reason about how modifiers combine. For example, if a class says "base", then it can be extended (anywhere). It would be extremely unfortunate if dartdoc broke this model by surfacing the transitive base requirements via syntax like sealed base, which would require users to do negative reasoning ("This class says base, but it can't be extended outside of the library because when you combine base with sealed it removes that ability").

@jcollins-g
Copy link
Contributor

"Inferred modifiers" will not be displayed as though they were real class modifiers to avoid this confusion. There may be some other indication that a class supertype is base but not by pretending a sealed class is sealed base.

@jcollins-g
Copy link
Contributor

This is now complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants