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

Add a header option to combining_builder #370

Open
natebosch opened this issue Aug 2, 2018 · 6 comments
Open

Add a header option to combining_builder #370

natebosch opened this issue Aug 2, 2018 · 6 comments
Labels
type-enhancement A request for a change that isn't a bug

Comments

@natebosch
Copy link
Member

The header option was removed from PartBuilder because individual builders no longer control the entire end file at .g.dart - it's now a concatenation of all .something.g.part files which might be produced by other builders as well. The ordering isn't controllable. Since we don't know which file will be first a "header" might not end up at the top of the resulting file.

We could add a header option to source_gen|combining_builder. I was originally opposed to doing this because:

  1. I only saw it being used for copyright headers and I didn't think that was a good justification.
  2. It's really unfortunate that a user who only knows about the package they are directly depending on would need to learn about a new builder they aren't specifically asking for in order to configure a header, and the usage of source_gen and the combining builder becomes an implementation detail that the end user depends on.

I've now heard a new use case which is adding //ignore_for_file: comments for opt in lints that the builder authors aren't aware of.

I still think (2) is a bummer but this is a pretty good reason to want a header. Technically these comment don't need to be at the top of the file and we could put them back as "section headers" in the individual parts which might end up anywhere in the result file, but I'm not sure that's much better than having to learn about a new builder name to configure the header on.

cc @kevmoo who was right about this from the beginning.

@natebosch
Copy link
Member Author

I'd also prefer if there were a better way to silence these lints in the analyzer. Filed dart-lang/sdk#34069 to see if we can avoid having to hack around lints in generated code like this.

@zoechi
Copy link

zoechi commented Aug 2, 2018

I also prefer configuring the analyzer, but because that dodn't work, using the header was a convemient workaround.

@davidmarne
Copy link

davidmarne commented Aug 2, 2018

Alternatively, built_value or any other code generator could still support lints via the build_config. Instead of passing those into source_gen's builder like built_value did here built_value could pass it to it's generator

@kevmoo
Copy link
Member

kevmoo commented Feb 13, 2020

This is blocking google/json_serializable.dart#557

@natebosch
Copy link
Member Author

This should not be blocking. I think you can drop an // ignore: anywhere in the file and it's fine to repeat it. It might not be pretty, but I wouldn't say it's blocking.

@kevmoo
Copy link
Member

kevmoo commented Feb 13, 2020

Really annoying?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants