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

CliRunner prompt input echos twice on Python 2 #1101

Closed
davidism opened this issue Aug 28, 2018 · 4 comments · Fixed by #1820
Closed

CliRunner prompt input echos twice on Python 2 #1101

davidism opened this issue Aug 28, 2018 · 4 comments · Fixed by #1820
Assignees
Labels
Milestone

Comments

@davidism
Copy link
Member

import click
from click.testing import CliRunner

@click.command()
def cli():
    prompt("value")
    click.echo("done")

runner = CliRunner(echo_stdin=True)
result = runner.invoke(cli, input="test")
print(result.output)

Python 3 output (correct):

value: test
done

Python 2 output:

value: testtest
done

I went back and this appears to be the case in every version since 1.x. The issue seems to be with the visible_input function in CliRunner.isolation. It uses input after that has been wrapped in EchoingStdin. It appears to work in Python 3 due to some weird lookup behavior, but that appears to be an accident. There are no tests for echo_stdin and prompt at the same time.

It didn't show up in the docs because the docs runner overrode isolation with different code that had visible_input pull from the original input and only replaced sys.stdin with the EchoingStdin wrapper, rather than both with the wrapper. While porting the docs runner to pallets-sphinx-themes, I removed that code in favor of simpler subclassing, and noticed that the docs had double input, which led me to discover the full issue.

@davidism davidism added the bug label Aug 28, 2018
@bporter125
Copy link
Contributor

Taking a look at this

@bporter125
Copy link
Contributor

So this feature has proven to be a little more complex than it would seem. The reason behind this bug is that the EchoingStdin was not being set up properly for Python 3 as it's intended functionality of printing anything being read is being overrode by io.TextIOWrapper:

if self.echo_stdin:
    input = EchoingStdin(input, bytes_output)	
input = io.TextIOWrapper(input, encoding=self.charset)

So some re-ordering of this can reproduce the double print for 3.7 as well:

input = io.TextIOWrapper(input, encoding=self.charset)
output = io.TextIOWrapper(
    bytes_output, encoding=self.charset)
if self.echo_stdin:
    input = EchoingStdin(input, output)
sys.stdout = output

Then removing the forced print will lead to what I would believe to the the expected functionality:

def visible_input(prompt=None):
    sys.stdout.write(prompt or '')
    val = input.readline().rstrip('\r\n')
-     sys.stdout.write(val + '\n')
+     sys.stdout.write('\n')
    sys.stdout.flush()
    return val

With echo_stdin=True both Python 2 and Python 3 output:

value: test
done

and echo_stdin=False outputs:

value: 
done

Assuming this is the desired functionality, this causes some breaks in unit tests, which to be honest are a little confusing as to what their intentions are based on how they are structured. Further investigation leads to a question of the actual purpose to the echo_stdin flag is, as there are also a hide_input and prompt options available to commands.

@ssbarnea
Copy link

This can be closed, support for py2 was removed.

@davidism
Copy link
Member Author

It revealed another issue with the Python 3 implementation.

@davidism davidism self-assigned this Apr 4, 2021
@davidism davidism added this to the 8.0.0 milestone Apr 4, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants