Conversation
| --all=false: | ||
| Include normally ignored files (VCS and editor special files). | ||
| -c, --config="": | ||
| -c, --config="reflex.conf": |
There was a problem hiding this comment.
This isn't the output of running reflex -h, is it?
You should update the -c documentation string to mention reflex.conf, particularly because it's not a normal flag default value.
There was a problem hiding this comment.
I would much appreciate some advice on how best to do that.
We can edit the help string to append the phrase Default: reflex.conf at the end, so it appears like so:
-c, --config="":
A configuration file that describes how to run reflex
(or '-' to read the configuration from stdin). Default: reflex.conf
But that's not how pflag prefers to indicate default arg values, instead this feels better:
-c, --config="reflex.conf":
A configuration file that describes how to run reflex
(or '-' to read the configuration from stdin).
However, unless I'm missing something, the 2nd method can only be done through the value arg of pflag.StringVarP call, which I don't want to do because there's a special case, where -c is not passed, but reflex.conf doesn't exist either. It's not simply that "default value for -c is reflex.conf" but rather "default value for -c is reflex.conf if the file exists in current working dir", and I'm at loss as to how best to handle that third case.
I'm leaning towards a spin on the first method above mentioned, like so:
-c, --config="":
A configuration file that describes how to run reflex
(or '-' to read the configuration from stdin).
Default: reflex.conf – if it exists in current working directory.
|
Bump for visibility. Question above. |
|
@cespare can you review please? Its been so long tho. |
Fixes #41
I was going through my issues list and saw that I had opened a feature request (2.5 years ago, sorry) for a "rc file" in the current directory, such that if it exists, it is automatically used if
--configflag is not set. I believe we settled on a straightforward name for the config file:reflex.conf. This PR implements that in a very simple, straightforward way:Before the config flag is processed, we check if it's empty and if
reflex.confexists. If it does, the flag is quietly set to that value before rest of the flag processing takes place. I also modified the wording of an error to indicate that the flag validation behavior is the same whether--configflag is specified or assumed (to be=reflex.conf).I tested this by building the reflex binary and running it in a directory with and without a
reflex.conffile and it seems to meet the expectations in this rudimentary, manual test. I'm not sure how to add an automated test for this. Contents ofmain()do not seem to be autotested anyway.I should also update the documentation before this is ready for review.
Edit: Also, prefer not to add my name to authors list. This is extremely trivial and I don't feel that I deserve the credit.