add 'hide-invalid-repositories' option
Hey. As I don't mind if Klaus find some invalid repositories, I added an option to hide those.
During the app building, when it formats the {in,}valid repositories, it checks for this newly created option and dismiss the invalid ones.
I hope that's enough for you! Thanks for this super software :)
Hey @jonashaag, could you please consider merging this PR or giving your opnion on it? Many thanks!
I’m sorry, I think I didn’t respond here because the tests fail and I wanted to wait for you to fix them before reviewing the PR
Review feedback: please add the new option at the end of the keyword arguments. Also I think it’s more obvious to only look at the hide flag where the invalid repos are rendered, not where the list of invalid repos is constructed.
Hey again, thank you for your time. I believe all is working this time!
Thanks a lot, please move the new setting to very end so as to make the changes backwards compatible.
Also what do you think about adding a "N invalid repositories hidden" to the repo list page? Do you think that could be helpful? Or maybe we can just log a warning to the terminal. Just a way for people to discover that some stuff has been hidden. Not sure what's the best option here.
move the new setting to very end
I could move it at the very end but the Klaus constructor takes ctags_policy as a keyword argument while it takes the others as positional. Making it the last makes the new option a keyword argument. I guess that's something you want.
Just a way for people to discover that some stuff has been hidden
Yes some stuff has been hidden but not by default, that's the user's will. I agree that both a normal logging as well as a small message to the repo list page could be useful. I'm on it :)
I just tried this, but when I start klaus --hide-invalid-repos some_invalid_repo, I still get klaus: error: argument DIR: 'some_invalid_repo': Not a Git repository. I was expecting the option to ignore that repo, or did I misunderstand what the option is supposed to do?
The added option won't change this behavior. You can't pass an invalid Git repository to klaus. This option is useful when you pass a folder containing directories, then the invalid ones aren't displayed.
But how do you pass a folder on the command line?
Using the standalone application (python bin/klaus) won't load an invalid repository and end with an error (klaus: error: argument DIR: <path>: Not a Git repository).
By wrapping Klaus within uwsgi, I believe you can make it works with some invalid repositories. That's how I use it daily with the Docker image.
I just investigated and found what causes that.
When running the standalone Klaus, the argument parser tries to match the passed directories with the git_repository function (./bin/klaus). An invalid repository will not match, raise an exception and cause the end of the run.
When wrapping using uwsgi (by example), it by-pass the initial argument type-check and sort the repositories between valid and invalid.
One way around is to disable this git-type check in the argument parser but that's not the goal of this PR. :)
EDIT: type check
But then the command line flag does not make a difference, does it?
I think it would be fine to have it disable the type check.
This commit disables the type check.
The command line flag doesn't make a difference with this previous type check as the repositories couldn't go "through" the type checker. Now it is disabled you can pass invalid repositories that will (or will not using this new option) get rendered.
Sorry, maybe wasn't clear enough – we can change behaviour only when --hide-invalid-repos is given, otherwise we're breaking backwards compat.
I don't think we can change behavior only when the option is given, that's all or nothing as the type check is done by the argument parser. If we really want to keep the Git type check, either we do it later or we keep this option as being useless when using the Python executable directly.
Yes, let's do the check later, for example in main where we do the other checks.
We could move the check in main but this can lead to 2 situations:
- nothing changes, the type check is done in
mainand the program still stops if an invalid repository is given (actual behavior) - the type check is done in every case (not only when running via
klausbinary directly), this would break backward compatibility by crashing the program when usinguwsgi
To be honest, I can't really understand the type check and why it is done.
On the repo list page, the invalid repositories are even displayed when using uwsgi, so why doing this test?
Hey @jonashaag, have you had some time to think about those type checks? :)