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

Jump to bottom

[WIP] Added --include and --exclude options to format tool #4635

Merged
merged 1 commit into from Apr 19, 2018
Merged

[WIP] Added --include and --exclude options to format tool #4635

merged 1 commit into from Apr 19, 2018

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Jun 29, 2017 •
edited

Solves #3267 by adding --include and --exclude options to ´tool format. exclude is by default populated with lib, so that files in ./lib folder will not be formatted. This can be enabled by providing --include lib as commandline option.

@straight-shoota straight-shoota changed the title Added --include and --exclude options to format tool [WIP] Added --include and --exclude options to format tool Jul 15, 2017
@straight-shoota
Copy link
Member Author

straight-shoota commented Jul 15, 2017 •
edited

There are still a few questions about this:

  • Should it be possible to specify absolute paths? Like crystal tool format --exclude /path/to/pwd/exclude_dir. If yes, normalization should turn every path into an absolute one with File.real_path. Or should absolute paths be banned or refer to current working directory?
  • Should globs be supported? This would enable e.g. to exclude all lib files from samples (which might be using their own shard dependencies) with samples/*/lib.
  • Directories like .git should probably be excluded by default.
  • Maybe it would even be better to use a fixed set of usual code locations like spec and src as default includes? This way, the formatter will not try to find Crystal files in other locations

@straight-shoota straight-shoota mentioned this pull request Jul 16, 2017
Closed
@sdogruyol
Copy link
Member

sdogruyol commented Apr 19, 2018 •
edited

I like the idea, this is good improvement over what we have. Let's KISS and have another PR for the remaining questions / improvements.

@RX14 RX14 modified the milestone: Next Apr 19, 2018
@sdogruyol sdogruyol merged commit 591580e into crystal-lang:master Apr 19, 2018
@sdogruyol sdogruyol added this to the Next milestone Apr 19, 2018
chris-huxtable pushed a commit to chris-huxtable/crystal that referenced this pull request Jun 6, 2018
@straight-shoota @chris-huxtable
Added --include and --exclude options to format tool (crystal-lang#4635)
ea7f464
@straight-shoota straight-shoota deleted the tool-format-exclude-option branch July 17, 2018 09:51
@RX14 RX14 mentioned this pull request Mar 16, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhass jhass jhass approved these changes

@sdogruyol sdogruyol sdogruyol approved these changes

@RX14 RX14 RX14 approved these changes

Assignees
No one assigned
Labels
kind:feature topic:tools:formatter
Projects
None yet
Milestone
0.25.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@straight-shoota @sdogruyol @jhass @RX14