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.

Sign up for GitHub

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

Jump to bottom

Fix console markup escaping issue #1866

Merged
merged 8 commits into from Feb 4, 2022
Merged

Fix console markup escaping issue #1866

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f6341de
fix for markup escaping in progress
willmcgugan Sep 14, 2021
6de69bc
whitespace
willmcgugan Sep 14, 2021
24d9bc9
restore new line
willmcgugan Sep 14, 2021
cb26c0a
Merge branch 'master' into master
tomchristie Oct 13, 2021
a30f832
Merge branch 'master' into master
tomchristie Oct 18, 2021
a2f3f07
Merge branch 'master' into master
Kludex Jan 23, 2022
75fb2ef
Merge branch 'master' into master
tomchristie Feb 4, 2022
973d1ed
Update _main.py
tomchristie Feb 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 9 additions & 6 deletions httpx/_main.py
View file
Open in desktop
Expand Up @@ -7,8 +7,10 @@
import pygments.lexers
import pygments.util
import rich.console
import rich.markup
import rich.progress
import rich.syntax
import rich.table

from ._client import Client
from ._exceptions import RequestError
Expand Down Expand Up @@ -173,20 +175,21 @@ def print_response(response: Response) -> None:

def download_response(response: Response, download: typing.BinaryIO) -> None:
console = rich.console.Console()
syntax = rich.syntax.Syntax("", "http", theme="ansi_dark", word_wrap=True)
console.print(syntax)

console.print()
content_length = response.headers.get("Content-Length")
kwargs = {"total": int(content_length)} if content_length else {}
with rich.progress.Progress(
"[progress.description]{task.description}",
"[progress.percentage]{task.percentage:>3.0f}%",
rich.progress.BarColumn(bar_width=None),
rich.progress.DownloadColumn(),
rich.progress.TransferSpeedColumn(),
) as progress:
description = f"Downloading [bold]{download.name}"
download_task = progress.add_task(description, **kwargs) # type: ignore
description = f"Downloading [bold]{rich.markup.escape(download.name)}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for this :)

I think this is the only relevant change in this PR wrt the "markup leak" issue, right? By only adding that rich.markup.escape() call I was able to prevent eg --download "[blink red]file.out" from rendering.

I'm not sure what the effect of the syntax and kwargs bits are, but they seem non-related, so they should be kept out, sounds legit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly the markup leak.

Replacing the kwargs was partly a simplification, but also covers the situation where there is no content length in the header. Consequently Rich will show a "in progress animation" rather than remain stuck at 0% until the download was finished.

download_task = progress.add_task(
description,
total=int(content_length or 0),
start=content_length is not None,
)
for chunk in response.iter_bytes():
download.write(chunk)
progress.update(download_task, completed=response.num_bytes_downloaded)
Expand Down