gh-127068: default REPL where prompts containing newlines would be dup #139465
Conversation
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
yihong0618
requested review from
pablogsal,
lysnikolaou and
ambv
as code owners
Lib/_pyrepl/reader.py
Outdated
| del screeninfo[num_common_lines:] | ||
|
|
||
| last_refresh_line_end_offsets = self.last_refresh_cache.line_end_offsets | ||
| last_refresh_line_end_offsets = self.last_refresh_cache.line_end_offsets.copy() |
There was a problem hiding this comment.
It seems this last_refresh_line_end_offsets has never been used in this function. The bug can be fixed by deleting this one.
The other two screen & screeninfo are synced back to self.last_refresh_cache. But for readability & maintenance, we should copy them here.
|
The failed Windows test ( If we add the - (4, 2)
+ (2, 3) : [(0, [1, 1, 1, 1, 1]), (0, [1, 1, 1, 1, 1]), (0, [1, 2, 1])] |
Signed-off-by: yihong0618 <zouzou0208@gmail.com> Co-authored-by: Keming <kemingy94@gmail.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
| offset, num_common_lines = self.last_refresh_cache.get_cached_location(self) | ||
|
|
||
| screen = self.last_refresh_cache.screen | ||
| screen = self.last_refresh_cache.screen.copy() |
There was a problem hiding this comment.
A copy is expensive if we do it at every refresh. Isn't there an alternate way to do it?
There was a problem hiding this comment.
for my first try copy is fine here, since its only in repl
and then will try to figure out if there is a better way.
thanks for the info
There was a problem hiding this comment.
The problem is that people want the REPL not to be slow in general. What's the maximum size of screen? if there are only a few string items (and if this is upper-bounded by the terminal size), then it should relatively be fine.
There was a problem hiding this comment.
One possible way is to keep using the reference instead of the copy, since these two variables will eventually be synced back to the self.last_refresh_cache. We can add some comments to explain this decision in the code to avoid misunderstanding for others.
The root cause of the this lis the calc_screen() method of reader.py. When screen, screeninfo, and line_end_offsets are retrieved from the cache, they are passed by direct reference instead of by a copy.
before this patch
2025-10-01.12.31.25.mov
after this patch
2025-10-01.12.57.17.mov