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

Allow to exclude nested namespaces #1586

Open
ghostbuster91 opened this issue Sep 17, 2024 · 8 comments · May be fixed by #1649
Open

Allow to exclude nested namespaces #1586

ghostbuster91 opened this issue Sep 17, 2024 · 8 comments · May be fixed by #1649

Comments

@ghostbuster91
Copy link
Contributor

When generating scala classes from smithy model we often need to exclude certain namespaces due to various reasons.

For that we were using --exclude-ns option in smithy4s-cli. However, recently we did some small refactoring that moved some of the shapes from the top namespace that was excluded (e.g. a.b) into sub-namespaces (e.g. a.b.c and a.b.d).

This cause issues in the generated code as those nested namespaces were not excluded.

Would it be possible to allow excluding namespaces in a more convenient way? e.g. --exclude-ns a.b.* would exclude .a.b as well as any other namespace that starts with a.b

@kubukoz
Copy link
Member

kubukoz commented Sep 17, 2024

I'm all for it, if it's simple to implement then it shouldn't be a big deal to @Baccata either I assume

@Baccata
Copy link
Contributor

Baccata commented Sep 30, 2024

PR away :)

@kubukoz
Copy link
Member

kubukoz commented Oct 22, 2024

@Baccata how flexible do you think this should be? Accept arbitrary regex, or just wildcards (if so, only trailing/leading or everywhere?)

@Baccata
Copy link
Contributor

Baccata commented Oct 22, 2024

just wildcards (glob patterns) would be fine. Whatever you can find a concise implementation for.

@kubukoz kubukoz changed the title Allow to exclude namespaces nestead namespaces Allow to exclude nested namespaces Oct 30, 2024
@msosnicki
Copy link
Contributor

msosnicki commented Feb 6, 2025

I can have a look into it. But first I would like to clarify the behavior.
Going with glob patterns, it would mean that we have:

  • a.b.** - matches all namespaces below a.b (but not a.b itself)
  • a.b.*.d - matches only a single level on a namespace

Exact namespaces can be excluded with a.b and the combination of * and ** gives a lot flexibility in other scenarios.

But the a.b.* could be treated more like regex (in which case dots would have to be escaped internally to mean dot literals), which would mean that it matches all of the below:

  • a.b
  • a.b.c
  • a.b.c.d

Not sure which is more preferable here. I like the first solution more, as it feels more precise. But can be cumbersome too, if I want to exclude all the shapes from a.b and nested namespaces: I would have to include two patterns: a.b and a.b.**.

@kubukoz
Copy link
Member

kubukoz commented Feb 6, 2025

IMO

  • a.b.* matches everything below a.b but not a.b itself
  • a.b* matches a.b and everything below

I don't think we have a need for * in the middle, it could be just a distraction at this point

so I guess I'm just proposing prefix-based exclusion/inclusion, with the special case of "no asterisk" which is an exact match

@msosnicki
Copy link
Contributor

Had a brief look, looks like alloy will also have to be involved https://github.com/disneystreaming/alloy/blob/6ac81f362362ea48165c07589021181e484f10ec/modules/openapi/src/alloy/openapi/package.scala#L33

I wonder why it only takes the allowedNS param, and the excludedNS is ignored in openapi context?

@Baccata
Copy link
Contributor

Baccata commented Feb 7, 2025

No particular reason, probably just an oversight

@msosnicki msosnicki linked a pull request Feb 12, 2025 that will close this issue
7 tasks
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 a pull request may close this issue.

4 participants