Skip to content

gh-153902: strip trailing newlines when displaying source in pdb#153905

Closed
saucoide wants to merge 3 commits into
python:mainfrom
saucoide:pdbnewlines
Closed

gh-153902: strip trailing newlines when displaying source in pdb#153905
saucoide wants to merge 3 commits into
python:mainfrom
saucoide:pdbnewlines

Conversation

@saucoide

@saucoide saucoide commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

The source lines pdb fetches from linecache.getlines & inspect.getsourcelines have trailing \n characters

passing them to _pyrepl.utils.disp_str turns them into ^J codes dirtying the output, pdb already prints line by line with end="\n" so rstripping each line shouldnt change the output

Maybe there is already a plan to whole switch pdb's output to the use the structured renderer from pyrepl, but in the meantime it looks broken

The source lines pdb fetches from linecache.getlines &
inspect.getsourcelines have trailing `\n` characters, passing them to
_pyrepl.utils.disp_str turns them into `^J` codes dirtying the output

pdb already prints line by line with end="\n" so rstripping each line
shouldnt change the output
Comment thread Lib/pdb.py Outdated
s += '>>'
if self.colorize:
line = self._colorize_code(line)
line = self._colorize_code(line.rstrip())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think e11465b is a better approach.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree that this is probably something pyrepl should fix. It should not just render \n to ^J unconditionally. Let's see if they'll fix that. If not, pdb can make its mitigation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the pyrepl change can be made in this PR, it's not a bug in the pyrepl's use-case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that does look better, @StanFromIreland do you know if hugo's change is going to be merged and this should wait for it, or we bring the change here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hugo’s change probably won’t be merged for a while, so we can do it here first.

CC @hugovk

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

brought the commit & updated this one

@ambv

ambv commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

I don't like this approach for this particular fix. Note that pdb already r-strips the line, only it does it after the colorizing. So a surgical fix is to just move the stripping pre-colorization. It's what I did in gh-154473 where I also focused on the more important piece of a PR like this: how to test this doesn't regress in the future.

Closing this PR, given that @hugovk's _pyrepl utils feature can (and will) be merged separately anyway.

@ambv ambv closed this Jul 22, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in Sprint Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants