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

Support passing directories as command-line arguments #64

Merged
merged 1 commit into from Aug 4, 2022

Conversation

pgjones
Copy link
Contributor

@pgjones pgjones commented Jul 21, 2022

This makes it easier to run djhtml over a bunch of files in a
directory or directories.

The default suffixes are used as a filter with no configuration
allowed (as black seems to work).

I'm aware of #13 recommending using find ... | xargs or the usage of pre-commit. However, I would like a single command that works on Linux, MacOS, and crucially Windows systems (find and xargs don't work). This would allow me to recommend to users how to use djhtml without having to add lots of additional text explaining how to do it on different systems i.e. djhtml src/templates is all that is needed.

@pgjones
Copy link
Contributor Author

pgjones commented Aug 3, 2022

@JaapJoris anything I can do to help this along?

Copy link
Member

@JaapJoris JaapJoris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for this valuable contribution! I especially like the clean and dependency-less implementation of _generate_files().

However, there is one use case that seems to break with this change, and that is indenting files that end with non-obvious (or absent) extensions. A good example are the files inside the tests/suite.

Before:

$ djhtml -ic tests/suite/html.in
1 template would have been reindented.

After

$ djhtml -ic tests/suite/html.in
0 templates would have been reindented.

Preserving this behavior is important to guarantee that DjHTML will always act upon the arguments it receives, no matter their extension. It's especially important for the folks that are using the files argument to pre-commit, a recent example being #65

If you are able to find an elegant solution to preserve the original behavior, while also adding in support for directory traversal, I will approve, merge, and release the PR. Good luck!

This makes it easier to run djhtml over a bunch of files in a
directory or directories.

The default suffixes are used as a filter with no configuration
allowed (as black seems to work).

The existing functionality whereby file names with any prefix can be
specified will continue to work.
@pgjones
Copy link
Contributor Author

pgjones commented Aug 4, 2022

That makes sense, I've updated and I believe it meets this requirement now.

@JaapJoris JaapJoris merged commit b40c0b8 into rtts:main Aug 4, 2022
@pgjones
Copy link
Contributor Author

pgjones commented Aug 4, 2022

Thanks :)

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 this pull request may close these issues.

None yet

2 participants