Conversation
…he server Reviewing what went in turned up three gaps. The editor waited for its lint, baseline and blame workers on close but not for the jedi completion thread, and the toolbar''s git-branch scan was never waited for at all -- both abort the process outright if they outlive what they hang off, and scanning branches on a large or network-mounted repository is not quick. Both are waited for now, and the remaining unnamed threads carry names so Qt''s abort message says which one. Document symbols were requested from the language server and parsed, but nothing connected the reply. The outline panel only ever parsed Python with ast, leaving every other language blank, so it now asks the server when ast finds nothing and nests what comes back by the depth the server reports.
GitService had no UI consumer at all -- the git panel talks to GitPython directly -- so the stash and conflict work landed in a module nothing called. The panel now reaches those through the service rather than growing a third copy of the same logic, with buttons for stashing, popping and resolving. Wiring them up surfaced a leak: an open Repo keeps long-lived git child processes, and GitService never closed one. It showed as an unrelated test timing out waiting for a thread to start -- dozens of stray child processes make even that slow. The service closes its repository, closes the previous one when opening another, and the panel closes both when it goes away, along with waiting for its own background threads.
Reviewing the new code again turned up two things. GitWorker was never instantiated anywhere: the panel has a worker of its own and the service layer is called directly. It is gone rather than left as a class that looks available but is not. The panel''s buttons and prompts were hard-coded English, while the language files have carried translations for exactly those strings all along -- the new stash and conflict buttons had followed the same pattern. They read from the dictionary now, which puts eighteen unused keys back to work and adds the ones the new buttons and dialogs need.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 88 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
A review pass over what went in with #248, and the fixes it turned up. Three
commits, all of them things the review found rather than new features.
Threads that outlived what they hang off
Qt aborts the process outright when a running
QThreadis destroyed, and themessage it prints is swallowed by pytest, so these show up as an unexplained
crash far from the cause.
its lint, baseline and blame workers but not this one -- the editor's only
thread without a manager looking after it.
main window, and scanning branches on a large or network-mounted repository is
not quick, so still running at closing time is not unusual.
operation such as fetch takes a while.
Every raw
QThreadnow carries a name, so if this ever happens again Qt'smessage says which one it was. That is how the toolbar thread was identified:
the message named it as soon as the names were in place.
This also corrects something said while fixing an earlier crash. The toolbar was
cleared of blame then, and the lint thread was indeed the dominant cause -- but
the toolbar does have the same defect, hidden behind the louder one.
Repositories that were never closed
GitServicenever closed itsRepo, and every open one keeps long-livedgit cat-filechild processes. It surfaced as an unrelated test timing outwaiting for a thread to start: dozens of stray child processes make even that
slow. The service closes its repository, closes the previous one when opening
another, and the panel closes both when it goes away.
Code that existed but could not be reached
nothing connected the reply. The outline panel only ever parsed Python with
ast, leaving every other language blank; it asks the server whenastfindsnothing and nests what comes back by the depth the server reports.
GitServicehad noUI consumer at all, since the git panel talks to GitPython directly. The panel
reaches them through the service now rather than growing a third copy of the
same logic.
GitWorkerwas never instantiated anywhere. It is gone rather than left asa class that looks available but is not.
Translations that were defined but unused
The git panel's buttons and prompts were hard-coded English while the language
files carried translations for exactly those strings. They read from the
dictionary now, which puts eighteen unused keys back to work.
Verification
ruff check .clean. 1712 tests pass on 3.11 and on 3.10.