-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Option to print help to std out #188
Comments
Thanks for this feedback. Let me think on this some and get back to you. |
One example to look at is Git also allows you to adjust the behavior with options ( Sources: https://www.git-scm.com/docs/git/1.7.4 and https://www.git-scm.com/docs/git-config |
Any progress on this enhancement? I'd be happy to do the work if @dbieber or other members have some guidance on how they'd like to see this feature implemented. It looks like @cbrochtrup may not be too far off on the implementation since At that point the environment functionality @nikking mentioned could be implemented in core. |
This is a feature I would really appreciate! I have been enjoying the simplicity of Fire so far, but when you have even a couple steps of nesting it quickly gets frustrating to keep bringing up a pager and having to remember all the arguments. I would love to have my history printed to std out so that I can refer back to it. Let me know if I can help at all! |
Here's how I think about this. We want all of the following:
So, for example, one API we could consider is:
Yes! Can you suggest an API that meets these 5 criteria? |
Just spitballin' here (this may be dumb) but could I'm willing to try this if you approve. |
Definitely not dumb :) What would be the signature of display_func? Would it accept an arbitrary object that it is responsible for both serializing and displaying? Or would it just accept a string and it is only responsible for displaying it? |
I was thinking it'd be identical to Now users could do def simple_display(lines, out):
text = '\n'.join(lines) + '\n'
print(text, out=out)
fire.Fire(display_func=simple_display) Something like this would switch between the user and default self._Display = Display
else:
# Code to make sure display_func is valid
self._Display = display_func and then call |
Serializing refers to going from the final component to the output string representing it, as in _PrintResult.
One thing to consider is that Display is only one of a few places that Fire displays output today. |
What if instead of passing in a function like class Display:
def serialize():
....
def print():
....
def error():
.... We would provide a couple of classes that users can import as follows: from fire.display import Display, Less, Stdout and then use as follows: fire.Fire(display=Stdout) This has the advantage of centralizing all of the display code, as well as making it clear what interfaces someone needs to implement if they want to build a custom output tool. Option to disable output altogether -- we let users pass in None Thoughts? |
A class like There appear to be at least four steps occurring to get something to print out; generation, serialization, formatting, and output. I think one of the main challenges is how tightly coupled the four steps are. The function I'm sure everyone else on this thread is way ahead of me but some of the questions that crossed my mind are: If you wanted to insert code to globally disable formatting, where would it go? Or, as is under discussion, if you wanted to change output destination, where would it go? As @dbieber pointed out, there isn't a central place. Separating out those three (since formatting is already separate) functional systems into discreet components seems like a good direction but also a reasonably challenging one. Some of the challenges I think would need to be solved to move this direction.
As more defined paths emerge in generation, serialization, formatting, and output then I think the spec for a scalable, generally applicable API will emerge. As the code evolves the main entrypoint might look something like this:
I may be over thinking the situation - Does this seem too deep of an approach for an otherwise simple problem? In the meantime, optionally shunting everything that would have got to the |
@jaredtrog I think you make some great points, but let me put my OOP hat on for a second to see if we can meet in the middle.
I was implying that class StdOut(Display):
# Other methods are implemented by DisplaY
def output():
....
I like this separation and think that whatever the solution it is, it should conform to this API.
One of the reasons that I'm suggesting a class instead of passing arguments like this is that it better supports customization. With an API like you've suggested above, what happens when I want to customize the entire display system? Do I need to implement a class for each step? What about customizing just formatting and output? Logically, all 4 of these steps -- generation, serialization, formatting, and output -- are conceptually very related, so I think it would be nice if we could handle them together. |
I think the middle is where the best solutions are found 👍. Let me restate what I think you're saying to make sure I'm responding to the ideas as stated.
Is that accurate? |
I like the idea of separating out these different components. Strawman ideaHere's the idea I'm toying with at the moment, but it still has some (big) drawbacks. Signature: Here's how you would do each of 1-4:
The signatures for both A few (maybe major) drawbacks:
Also, I think we would omit Thoughts on configuring with a classOne concern I have about using a class for configuration is that the amount of boilerplate is high (at least by Fire's standards). A user would have to import a base class, create a new class, create a new function, and then include their (likely one-liner) function definition. Another concern is that it makes the API less visible. So far the API is visible by looking at (Tying this back to the 5 criteria above, I think using a Display class for configuration risks harming goal 5 of having a clean api.) Signature of config functionsHere's another thing I'm thinking about: Let's say we accept a function |
@jaredtrog That's exactly what I was suggesting, thanks for putting it so succinctly. @dbieber I understand your concerns, but I think that any solution we decide upon is going to increase the complexity of the API, since we're trying to handle a few new usecases as well as allow users to implement arbitrary functionality. That being said, I think one of the best parts of Fire is its minimalist design/API, so we should definitely keep our solution as sparse as possible. One concern I have with just placing functions into the I'm also not sure I understand the distinction between |
Agreed that's another drawback to the strawman proposal. It would definitely be good to have a solution that allows turning off all output with a single setting. |
That suggests another candidate: def display(text, file=None, kind=None):
...
fire.Fire(display=display) where It would be nice to trim this down to just |
I like that proposal and think that if the user is writing a custom display function, it's ok for them to conform to our expected API (naming args file/kind). Do we need to account for the serialization usecases as well though? |
Yeah, serialization would be a separate config under this proposal. def serialize(component):
return repr(component) # or whatever the user decides
fire.Fire(serialize=serialize) |
Maybe also we would use [An advantage of DefaultDisplay/DefaultSerialize though is that it allows Fire to offer built in alternatives if we later want to (like fire.SerializeCompact or something...) though idk if we'd want that. I guess fire.Serialize doesn't preclude that either...] |
Sorry, I think I'm getting a bit confused here. Are I think |
Functions |
fire.Fire is a function too. |
The reason we use FunctionNames instead of function_names is that at the time this project was created, that was an acceptable standard by the internal google python style guide. We've continued doing it for consistency. It would be good to switch to the more accepted standard of snake_case |
Ok, I'm still a little worried about the fact that we will need to add an argument per operation. What happens when we want to add a step between serialization and display (e.g. formatting)? |
Let's do our best to anticipate things we may want to add in the future. Can you give an example of what formatting might be used for? To me it sounds like serialization and display cover the full pipeline from output to eyeball so I don't think a separate formatting step would be needed. |
[Brainstorming: Another somewhat related place we may end up wanting an additional argument is exception handling. Fire raises FireExit exceptions (which extend SystemExit) in order to exit with a particular exit code. Users can catch these exceptions to prevent Fire from exiting the program, but it's possible we'd want to add a config around this too.] |
Maybe I misunderstand what serialization entails (I would have thought it involves taking the information to be output and packaging it into a single, easily transmitted bundle). Formatting to me would then be the process of arranging that information on screen (and display would be where to show it). An example of formatting would be if you're displaying on a pager, you might want to format with extra line breaks between arguments to improve readability. Or that you might want to list the parameter shortcuts (-x) in a different place from the full parameter names (--xample) |
Coming from #231, I think the proposed
when called like
would print Also not related to #231, IMHO I'd rather pass functions to control output than classes, since they can be written on the spot, but I'm not familiar at all with the internals. |
As a note, until there's a proper fix, the following workaround does seem to work (on version
|
Just wanted to throw another use case in here: I had to disable Display because it's printing out the result of my command (a very large dict) as a man-page style output (which doesn't make any sense in my context). |
A variant of @fabioz’s excellent workaround for people who prefer one-liners: if __name__ == "__main__":
# Make Python Fire not use a pager when it prints a help text
fire.core.Display = lambda lines, out: print(*lines, file=out)
fire.Fire(...) |
Improve the Cookiecutter template in the following ways: - Make hello function echo the input - Add test for echoing the input - Add support for Python Fire - Add workaround for known issue [1] in Python Fire - Add workaround for an issue in the Code Runner extension - Add tasks for Visual Studio Code: - Show usage help (only if Python Fire is enabled) - Run hello - Run hello with an argument - Code Runner: ignore editor selection - Add option to skip installing dependencies [1] google/python-fire#188 (comment)
I learned the export PAGER=cat Unfortunately, it will also change the output of other programs, like Maybe something like this in Fire? With error checks for valid binaries. fire_pager = os.environ.get("FIRE_PAGER")
if fire_pager:
os.environ["PAGER"] = fire_pager This looks clunky to me but seems easy to implement. |
If probably would be better to check See also python-fire/fire/console/console_io.py Line 85 in c378906
|
That is a much better idea. I confess, I didn't look at the code much before commenting 😅 I'll try to PR this soon but if someone has time, by all means, go ahead. |
This method is nice except the colors is missing. |
I ran into formatting issues so had to modify a bit, combining @fabioz and @liudonghua123 solution if __name__ == "__main__":
# Make Python Fire not use a pager when it prints a help text
fire.core.Display = lambda lines, out: out.write("\n".join(lines) + "\n")
fire.Fire(...) |
Usually with bash you get a simple, condensed version of the help when using the |
I found the paging feature is enabled default for bat, see https://github.com/sharkdp/bat?tab=readme-ov-file#automatic-paging. However, with So maybe there is an opportunity which supports both no-paging and colors output in python-fire. And I also find some interesting features like |
Thanks for creating Fire. It's a fantastic argument parsing tool.
A minor request: I prefer my help strings to be printed to stdout instead of a console like
less
. When the output is sent to aless
-like program you can't reference the script argument information when trying to type your next command.The naive way to allow this option is adding a switch so the user can choose to send information console package or to stdout (code below).
python-fire/fire/core.py
Line 171 in d774539
to something like
I don't see an elegant way to set
print_std
in your source code since you are not accepting contributions to your console package and theDisplay
function is not a method of the Fire class. I don't see a straightforward way to have the user set this display option. Obviously, you know the code better than me so you may know a good way to add this option.I am using fire version 0.2.1 and have seen this behavior with Ubuntu 16.04 and 18.04.
Thank you for your time!
The text was updated successfully, but these errors were encountered: