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

Feature Request: flag to limit languages processed #687

Closed
robfig opened this issue Dec 27, 2019 · 4 comments · Fixed by #755
Closed

Feature Request: flag to limit languages processed #687

robfig opened this issue Dec 27, 2019 · 4 comments · Fixed by #755

Comments

@robfig
Copy link
Contributor

robfig commented Dec 27, 2019

I have the following scenario:

  • I have a Gazelle binary that composes rules for JS, Go, and Proto
  • I want to process only Go rules under some/path
  • I want to process only JS rules under other/path.

Right now, there's no way to do that. If I run this Gazelle binary against our Go codebase, it will produce undesirable rules for any JS files checked in there.

I want to propose a new flag, --lang, which is a comma separated list of languages. It defaults to empty string, which has no effect.

It would be implemented as a flag in updateReposConfigurer, filtering based on language.Language.Name().

func updateRepos(args []string) error {
	cexts := make([]config.Configurer, 0, 2)
	cexts = append(cexts, &config.CommonConfigurer{}, &updateReposConfigurer{})
	kinds := make(map[string]rule.KindInfo)
	loads := []rule.LoadInfo{}
	for _, lang := range languages {
		// filter here
		loads = append(loads, lang.Loads()...)
		for kind, info := range lang.Kinds() {
			kinds[kind] = info
		}
	}

I think that's all it would take, although I haven't tested it out.

If this sounds good to you, I'll put together a PR.

Thanks!

@jayconrod
Copy link
Contributor

How are you invoking Gazelle usually?

This seems reasonable for people invoking Gazelle manually (without bazel run). Also useful for people passing additional arguments to bazel run.

For people using the gazelle rule only, you'd need multiple gazelle rules, so it's possible to also have multiple gazelle_binary rules with different sets of languages. This proposal seems a little redundant with that capability, though still useful.

@robfig
Copy link
Contributor Author

robfig commented Jan 2, 2020

I'm invoking it via bazel rules. I guess the real reason I'm interested in this feature is that Gazelle logs errors on unrecognized directives. Running it for Go logs ~20 lines about JS directives that it doesn't recognize.

@jayconrod
Copy link
Contributor

Ah, that makes sense. That does seem like a good reason to have a -lang flag.

It should probably be a flag in config.CommonConfigurer rather than updateReposConfigurer since it would need to be understood by all subcommands.

robfig pushed a commit to robfig/bazel-gazelle that referenced this issue Apr 10, 2020
This adds a `-lang` flag and `gazelle:lang` directive to restrict the languages
composed by Gazelle.

Fixes bazel-contrib#687
@robfig
Copy link
Contributor Author

robfig commented Apr 10, 2020

Sorry for the delay -- PR is up. I also added it as a directive, because I recall you saying that flags should generally be overridable by directives. I can remove one or the other if you prefer, either mechanism works for my use case.

robfig pushed a commit to robfig/bazel-gazelle that referenced this issue May 6, 2020
This adds a `-lang` flag and `gazelle:lang` directive to restrict the languages
composed by Gazelle.

Fixes bazel-contrib#687
robfig pushed a commit to robfig/bazel-gazelle that referenced this issue May 7, 2020
This adds a `-lang` flag and `gazelle:lang` directive to restrict the languages
composed by Gazelle.

Fixes bazel-contrib#687
jayconrod pushed a commit that referenced this issue May 7, 2020
This adds a `-lang` flag and `gazelle:lang` directive to restrict the languages
composed by Gazelle.

Fixes #687
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants