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

Allow debug_stream to be set optionally #393

Closed
azjps opened this issue Dec 17, 2022 · 1 comment · Fixed by #471
Closed

Allow debug_stream to be set optionally #393

azjps opened this issue Dec 17, 2022 · 1 comment · Fixed by #471

Comments

@azjps
Copy link

azjps commented Dec 17, 2022

First, a thank you for the project; I've been looking into integrating argcomplete into ipython/traitlets#811 so that shell completion works "out-of-the-box" for IPython, Jupyter, and everything else in the ecosystem.

Currently in CompletionFinder.__init__(), the following is done:

        global debug_stream
        try:
            debug_stream = os.fdopen(9, "w")
        except Exception:
            debug_stream = sys.stderr
        debug()

Could this behavior be made optional? I think it could either be extracted out into a separate helper method that can be overridden, or perhaps be controlled by either an additional argument (similar to output_stream=) or environment variable. Feel free to let me know which (or both) is preferable, and I can open a PR.

The problem is that this file descriptor could be used elsewhere, for example by pytest, leading to obscure failures. You can find a MRE in ipython/traitlets@2654e3f. I was able to get around this for now by mock-ing os.fdopen ipython/traitlets@0deadcc, but I'd prefer not to have to introduce mocking if possible.

I think this was also identified as one of two issues in #365 but was closed since the other half was resolved.

azjps added a commit to azjps/argcomplete that referenced this issue Dec 19, 2022
Allow debug_stream to be optionally set in CompletionFinder.__call__.
By default, this is still set to os.fdopen(9), but now it can be
overridden to a different i/o stream instead. May be useful for
unit tests, especially as os.fdopen(9) is likely to conflict
with pytest plugins and lead to obscure errors.

Closes kislyuk#393
@kislyuk
Copy link
Owner

kislyuk commented Mar 19, 2023

Thanks for looking into this. I am open to making the debug stream initialization optional by breaking it out into a method that can be disabled in subclasses. Would you be interested in creating a PR for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants