-
Notifications
You must be signed in to change notification settings - Fork 5
fix: close terminal tab when thread is deleted #54
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.
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
base: main
Are you sure you want to change the base?
Conversation
Store terminal tab reference at creation time and use it directly when closing, instead of searching by name which could fail if tab.label doesn't match terminal.name. Changes: - Add terminalTabs Map to store tab references - Save tab in createTerminal() right after creation - Use stored tab in closeTerminal() for reliable closing - Clean up tab reference in unregisterTerminal() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @eatnug, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 스레드가 삭제될 때 관련 터미널 탭이 자동으로 닫히지 않던 버그를 수정합니다. 기존에는 터미널 탭을 이름으로 찾아 닫으려 했으나, 이는 불안정할 수 있었습니다. 이제 터미널 생성 시 해당 탭의 참조를 직접 저장하고, 스레드 삭제 시 이 참조를 사용하여 정확하고 안정적으로 탭을 닫도록 개선되었습니다. 이로써 사용자 경험이 향상되고 불필요하게 열려있는 터미널 탭이 줄어들게 됩니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
이 PR은 스레드 삭제 시 터미널 탭이 닫히지 않는 버그를 수정합니다. 터미널 이름으로 탭을 찾는 대신, 생성 시점에 탭 객체를 저장해두고 이를 직접 참조하여 닫는 방식으로 안정성을 높인 점이 좋습니다.
전반적으로 좋은 수정이지만, 몇 가지 개선점을 제안합니다. 첫째, 터미널 생성 직후 탭을 찾는 과정에서 발생할 수 있는 레이스 컨디션을 해결하기 위한 방안을 제안했습니다. 둘째, 코드의 명확성을 높이기 위해 중복된 리소스 정리 로직을 제거하는 것을 제안했습니다. 자세한 내용은 각 주석을 참고해주세요.
| if (!openInPanel) { | ||
| const tab = this.findTerminalTab(name); | ||
| if (tab) { | ||
| this.terminalTabs.set(terminalId, tab); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vscode.window.createTerminal API 호출 후 UI에 탭이 즉시 생성되지 않을 수 있어 race condition이 발생할 수 있습니다. findTerminalTab(name)이 탭을 찾지 못하면 terminalTabs에 저장되지 않아, 나중에 터미널을 닫을 때 탭이 닫히지 않는 버그가 재현될 수 있습니다.
이 문제를 해결하기 위해, 탭을 찾을 때 짧은 지연을 두고 몇 차례 재시도하는 방식을 사용하는 것이 좋습니다. createTerminal의 반환을 지연시키지 않도록 이 로직은 즉시 실행 함수 표현식(IIFE)을 사용하여 비동기적으로 처리할 수 있습니다.
if (!openInPanel) {
// The tab may not be available immediately. We'll try to find it asynchronously
// with a short delay and retries to mitigate a potential race condition.
(async () => {
for (let i = 0; i < 5; i++) {
const tab = this.findTerminalTab(name);
if (tab) {
this.terminalTabs.set(terminalId, tab);
return;
}
await new Promise(resolve => setTimeout(resolve, 100));
}
this.log(`Could not find tab for terminal ${name} (${terminalId}) after creation.`);
})();
}| } catch (err) { | ||
| this.log(`Failed to close terminal tab: ${err}`); | ||
| } | ||
| this.terminalTabs.delete(terminalId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
closeTerminalTab 메서드에서 terminalTabs 맵의 항목을 삭제하고 있는데, 이 메서드를 호출하는 closeTerminal 메서드에서 잠시 후 unregisterTerminal을 호출하며, 여기서도 동일한 항목을 삭제하고 있습니다.
unregisterTerminal이 터미널 관련 모든 리소스를 정리하는 역할을 하므로, 리소스 정리는 한 곳에서만 처리하는 것이 좋습니다. closeTerminalTab에서는 탭을 닫는 역할에만 집중하도록 하고, terminalTabs에서 항목을 삭제하는 로직은 unregisterTerminal에 남겨두는 것이 코드 구조상 더 명확합니다.
Summary
Changes
terminalTabsMap 추가하여 탭 참조 저장createTerminal()에서 생성 직후 탭 저장closeTerminal()에서 저장된 탭으로 직접 닫기unregisterTerminal()에서 탭 정리Test plan
🤖 Generated with Claude Code