Conversation
… you can see the name and pid of the process name you are looking up. It can be a bit redundant, but it's in witr, so hey
WalkthroughAdded GetProcessNameFromPid(DWORD) to resolve an executable name from a PID via process snapshot enumeration. PIDinspect(DWORD) was refactored to use this helper and now prints Target/Process output (with VT100 awareness). Redundant early PID/process prints were removed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ation this fixes syntax error btw lol
fluffing up my commit messages is so fun so that the release looks good
…move redundant output
|
@coderabbitai wow hah this might be my record for a single feature, only took 4 commits!!! (well you haven't reviewed yet so i can't be too sure if there aren't gonna be more but hey i can see it working) |
|
Let me dive into the changes and see what you've built. Fingers crossed it's smooth sailing, but either way, it's great that you've got it working! 🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@main.cpp`:
- Around line 380-400: GetProcessNameFromPid currently returns
std::to_string(pid) when no process match is found, causing PIDs to be displayed
as process names; change the function to return an empty string on miss (keep
the existing WideToString(pe.szExeFile) return when found and the
INVALID_HANDLE_VALUE early return) so callers can render "N/A" or otherwise
handle missing names; update any call sites that rely on the PID-as-name
behavior to treat an empty string as "not found".
|
@coderabbitai welp I did speak too soon lol |
|
Tip For best results, initiate chat on the files or code changes.
The important thing is that you're making solid progress on the feature. Keep it up! 🚀 🧠 Learnings used |
If the PID isn’t found in the snapshot, returning std::to_string(pid) makes PIDinspect print the PID as the process name, which is misleading for nonexistent processes. Now it's fixed
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@main.cpp`:
- Around line 380-400: In GetProcessNameFromPid, fix the typo in the snapshot
failure comment: replace "vroken" with "broken" so the comment reads correctly
when CreateToolhelp32Snapshot returns INVALID_HANDLE_VALUE; keep the function
logic and resource cleanup (CloseHandle) as-is.
🧹 Nitpick comments (1)
main.cpp (1)
586-601: LGTM with minor style suggestions.The integration with
GetProcessNameFromPidis clean, and the N/A fallback for missing processes is handled correctly in both VT100 and non-VT100 paths.Optional nits:
- Prefer
procName.empty()overprocName == ""for idiomatic C++- Line 592 has a redundant
\033[0mafterprocName(no color was applied to it)♻️ Optional cleanup
std::string procName = GetProcessNameFromPid(pid); if (IsVirtualTerminalModeEnabled()) { - if (procName == ""){ + if (procName.empty()){ std::cout << "\033[34mTarget:\033[0m N/A\n\033[34mProcess:\033[0m N/A\n"; } else { - std::cout << "\033[34mTarget:\033[0m " << procName << "\033[0m" << std::endl; + std::cout << "\033[34mTarget:\033[0m " << procName << std::endl; std::cout << "\033[34mProcess:\033[0m " << procName << "\033[90m (pid " << std::to_string(pid) << ")\033[0m" << std::endl; } } else { - if (procName == ""){ + if (procName.empty()){ std::cout << "Target: N/A\nProcess: N/A\n"; } else { std::cout << "Target: " << procName << std::endl;
resolves #57
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.