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

Handle non text in style/secho by converting to text #1146

Merged
merged 2 commits into from Aug 8, 2020

Conversation

segevfiner
Copy link
Contributor

@segevfiner segevfiner commented Oct 14, 2018

Call str() on any non-string type. echo() also supports printing bytes, but this PR doesn't support that, ending up printing its repr (b'bytes'). I'm not sure how to handle it, since making style() support bytes might be rather ugly.

@davidism
Copy link
Member

davidism commented Aug 8, 2020

Within echo, there's already a check to not try to strip bytes of color information. This makes sense, as it wouldn't make sense to color raw bytes being output (even if they do represent text). So I think it makes sense to only do str(message).

That said, I did try an implementation for decoding bytes passed to secho, but decided against it. It had to be in secho and not style since only secho has information about the stream being written to.

from ._compat import get_best_encoding

# extracted from echo for use in secho as well
def _get_echo_file(file, err):
    if file is not None:
        return file

    if err:
        return _default_text_stderr()
    else:
        return _default_text_stdout()

def secho(message, file=None, err=False, ...):
    file = _get_echo_file(file, err)

    if message is not None:
        if is_bytes(message):
            file = _get_echo_file(file, err)
            encoding = get_best_encoding(file)
            message = message.decode(encoding)
        elif not isinstance(message, str):
            message = str(message)

        message = style(message, **styles)

    return echo(message, file=file, err=err, ...)

This isn't completely correct, it doesn't pull out the errors attribute of the stream for decoding. In the end, this seemed a bit too complicated. Since 8.0 is Python 3-only, it also makes more sense to require text for output, requiring users to explicitly decode bytes being passed is the common pattern now.

@davidism
Copy link
Member

davidism commented Aug 8, 2020

I don't think this addresses #944, that seems to be about allowing ClickException.show() to use secho() instead of echo(), not being able to pass other types to secho().

@davidism davidism added this to the 8.0.0 milestone Aug 8, 2020
@davidism
Copy link
Member

davidism commented Aug 8, 2020

I guess we could also do not is_bytes(message) before applying style() in secho(). That way bytes get passed directly to echo().

@davidism davidism merged commit 77cb257 into pallets:master Aug 8, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants