Skip to content

Event.raw being added to itself instead of to another event's .raw in UnixConsole.getpending #145886

Description

@devdanzin

Bug report

Bug description:

There is a small bug in _pyrepl.unix_console.UnixConsole.getpending, where instead of adding the raw data of event 2, we add the raw data of event 1 to itself:

e = Event("key", "", b"")
while not self.event_queue.empty():
e2 = self.event_queue.get()
e.data += e2.data
e.raw += e.raw

We can just fix the typo: e.raw += e.raw becomes e.raw += e2.raw. It's feasible to add a test for this.

But Event.raw isn't read anywhere and is untested. The 3 callers of getpending() only use ev.data or pending.data. The .raw field is only written to, never consumed. So we can simply remove it from Event and adapt some lines (summary below written with Claude Code).

Please let me know which solution is preferred and I can write a trivial PR to implement it.


Since .raw is never read by any consumer, it's dead code. This removes the field and all assignments to it:

console.py — Remove raw from the dataclass:

@dataclass
class Event:
    evt: str
    data: str

unix_console.py — Remove all raw-related code in both getpending variants:

  • Line 540: Event("key", "", b"") → Event("key", "")
  • Line 545: delete e.raw += e.raw
  • Line 549-550: raw = ... / data = str(raw, ...) → data = str(self.__read(amount), ...)
  • Line 552: delete e.raw += raw
  • Same changes for lines 564, 569, 572-575

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

Activity

  1. added
    type-bugAn unexpected behavior, bug, or error
    stdlibStandard Library Python modules in the Lib/ directory
    topic-replRelated to the interactive shell
    on Mar 12, 2026
  2. added a commit that references this issue on Mar 12, 2026
  3. chris-eibl commented on Mar 13, 2026

    @chris-eibl
    Member

    Since .raw is never read by any consumer, it's dead code. This removes the field and all assignments to it:

    Yeah, I already did so for Windows in adb89ba and left a smiley on that typo #132440 (comment).

    Tbh, I think it is time to remove .raw altogether instead of fixing the wrong-but-dead code path.

    The only thing I am concerned is the side effect of flush_buf in

    self.insert(Event('key', k, bytes(self.flush_buf())))

    and
    self.insert(Event('key', decoded, bytes(self.flush_buf())))

    but I think, we can just simplify

    def flush_buf(self):
        """
        Flushes the buffer.
        """
        self.buf = bytearray()

    since the raw member is nowhere needed AFAICT ...

  4. chris-eibl commented on Mar 13, 2026

    @chris-eibl
    Member

    Please let me know which solution is preferred and I can write a trivial PR to implement it.

    @wavebyrd please don't open a PR when

    • the OP already stated they're up for it
    • and furthermore there even isn't a decision, yet
  5. deleted a comment from khalidsaidi on Mar 14, 2026
  6. added a commit that references this issue on Mar 18, 2026
  7. HCYT commented on Mar 18, 2026

    @HCYT

    Hi, I opened a PR for this: #146097

    I went with removing the Event.raw field entirely since it's not read anywhere, instead of just fixing the typo. Also cleaned up the tests.

    @chris-eibl would appreciate if you can take a look when you have time, thanks!

  8. picnixz commented on Mar 18, 2026

    @picnixz
    Member

    We did not decide whether to remove the field or not. Please do not open PRs

  9. picnixz commented on Mar 18, 2026

    @picnixz
    Member

    I think we need to keep the raw data. I might be necessary in the future to handle special key sequences that are not recognized. And if we were to expose the repl publicly it would also be a way for custom handlers to handle the event.

    So I personally prefer fixing the typo rather than removing the field.

  10. picnixz commented on Mar 18, 2026

    @picnixz
    Member

    OTOH the Windows console has no more raw buffer. What was the rationale for removing it?

  11. chris-eibl commented on Mar 18, 2026

    @chris-eibl
    Member

    I didn't remove it, just not populate it anymore in the few code parts that did, while most did not, and there was no "user" of it.

  12. picnixz commented on Mar 18, 2026

    @picnixz
    Member

    So there was an inconsistency in how it is being populated you mean? if this the case, then ok for removing the field altogether. However could you check if pypi repos use the pyrepl private implementation? while clearly internal we should avoid breaking packages when possible (at least give them a heads up)

  13. devdanzin commented on Mar 18, 2026

    @devdanzin
    MemberAuthor

    I'm not too attached to writing the PR, I just thought it would help. So feel free to reopen the PR that matches the agreed decision when one is made.

    Thank you @HCYT and @wavebyrd for the interest and willingness to help, we just need to do things in an orderly way, like reaching a decision before firing up the PRs.

  14. chris-eibl commented on Mar 20, 2026

    @chris-eibl
    Member

    So there was an inconsistency in how it is being populated you mean?

    For Windows, yes. And just not populating Event.raw anymore at all in adb89ba about a year ago seems to have not caused any pain so far.

    However could you check if pypi repos use the pyrepl private implementation?

    Ok. I've used @vstinner's search_pypi_top.py and searched for _pyrepl in the top 5000 packages, because console or Event would give too many false-positives. Stripping stdlib_list-0.12.0.tar.gz hits from the output results in only 37 occurrences

    Details

    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: prefer_pyrepl = True
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: using_pyrepl = False # overwritten by find_pyrepl
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: def find_pyrepl(self):
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: import _pyrepl.completing_reader
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: import _pyrepl.readline
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: sys.modules["readline"] = _pyrepl.readline
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: self.using_pyrepl = True
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: return _pyrepl.readline, True
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: self.using_pyrepl = True
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: if self.prefer_pyrepl:
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: result = self.find_pyrepl()
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: readline.backend = "_pyrepl"
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: readline._setup(namespace or {}) # internal _pyrepl implementation
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: if config.using_pyrepl or sys.platform != "darwin":
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: def interact_pyrepl(namespace: dict | None = None):
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: from _pyrepl import readline
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: from _pyrepl.console import InteractiveColoredConsole
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: from _pyrepl.simple_interact import run_multiline_interactive_console
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: if completer.config.using_pyrepl and "pypy" not in sys.builtin_module_names:
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/fancycompleter/init.py: interact_pyrepl(namespace)
    .\fancycompleter-0.11.1.tar.gz: fancycompleter-0.11.1/pyproject.toml: module = ["_pyrepl.", "pyrepl.", "pyreadline.*"]
    .\mpmath-1.4.1.tar.gz: mpmath-1.4.1/mpmath/main.py: from _pyrepl.main import CAN_USE_PYREPL
    .\mpmath-1.4.1.tar.gz: mpmath-1.4.1/mpmath/main.py: from _pyrepl.console import
    .\mpmath-1.4.1.tar.gz: mpmath-1.4.1/mpmath/main.py: from _pyrepl.main import CAN_USE_PYREPL
    .\mpmath-1.4.1.tar.gz: mpmath-1.4.1/mpmath/main.py: from _pyrepl.simple_interact import
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/src/pdbpp.py: def _patch_readline_for_pyrepl(self):
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/src/pdbpp.py: uses_pyrepl = self.fancycompleter.config.readline != sys.modules["readline"]
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/src/pdbpp.py: if not uses_pyrepl:
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/src/pdbpp.py: with self._patch_readline_for_pyrepl():
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/testing/conftest.py: ("pyrepl" if sys.version_info < (3, 13) else "_pyrepl"),
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/testing/conftest.py: m.setattr("fancycompleter.DefaultConfig.prefer_pyrepl", False)
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/testing/conftest.py: import _pyrepl.readline
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/testing/conftest.py: m.setattr("fancycompleter.DefaultConfig.prefer_pyrepl", True)
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/testing/conftest.py: elif readline_param == "_pyrepl":
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/testing/conftest.py: readline = "_pyrepl.readline"
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/testing/test_pdb.py: # and readline_param != "_pyrepl"
    .\pdbpp-0.12.1.tar.gz: pdbpp-0.12.1/testing/test_pdb.py: prefer_pyrepl = False

    where the only line to investigate further

    .\mpmath-1.4.1.tar.gz: mpmath-1.4.1/mpmath/__main__.py: from _pyrepl.console import \
    

    reveals that
    https://github.com/mpmath/mpmath/blob/7b40a40cc6b8961cfa9d1697e6e7075d42f56f0a/mpmath/__main__.py#L113-L114

                    from _pyrepl.console import \
                        InteractiveColoredConsole as InteractiveConsole

    is harmless, too. So I could definitely not find a direct user of Event.raw. To me, the hits do not even hint for any indirect users, but I might have overlooked some code paths.

  15. chris-eibl commented on May 25, 2026

    @chris-eibl
    Member

    @pablogsal has fixed the type in getpending() in #146584.
    I think we can close here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    stdlibStandard Library Python modules in the Lib/ directorytopic-replRelated to the interactive shelltype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions