Skip to content

target entry#58

Merged
supervoidcoder merged 5 commits intomainfrom
target-entry
Jan 29, 2026
Merged

target entry#58
supervoidcoder merged 5 commits intomainfrom
target-entry

Conversation

@supervoidcoder
Copy link
Owner

@supervoidcoder supervoidcoder commented Jan 29, 2026

resolves #57

Summary by CodeRabbit

  • New Features

    • Process display now resolves and shows executable names alongside PIDs; shows "N/A" when a name can't be determined.
    • Output adapts to terminal capabilities for clearer VT100-aware formatting.
  • Refactor

    • Removed redundant early prints and standardized process information presentation for consistent, less cluttered console output.

✏️ Tip: You can customize this high-level summary in your review settings.

… 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
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 29, 2026

Walkthrough

Added 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

Cohort / File(s) Summary
Process name resolution & PID output
main.cpp
Added std::string GetProcessNameFromPid(DWORD pid) which snapshots processes and returns the executable name (or PID-as-string / empty on failure). Refactored PIDinspect(DWORD) to call this helper, updated VT100 vs non-VT printing paths, and removed redundant early prints of PID/process name at call sites. Minor formatting/comment tweaks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A rabbit sniffed a PID trace,
I found its name and saved its place.
No double prints, the output's clean,
A neat small helper—quick and keen. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 3
❌ Failed checks (1 warning, 2 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'target entry' is vague and generic, using non-descriptive terms that don't clearly convey what the pull request accomplishes. Revise the title to be more specific and descriptive, such as 'Add process name lookup from PID' or 'Refactor process inspection with process name resolution'.
Linked Issues check ❓ Inconclusive The pull request appears to address issue #57 by implementing process name lookup functionality from PIDs, but the linked issue has no description available for detailed validation. Verify that the process name lookup implementation in GetProcessNameFromPid() and its integration in PIDinspect() fully addresses the requirements specified in issue #57.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Out of Scope Changes check ✅ Passed All changes are focused on implementing process name lookup functionality and refactoring PIDinspect() output, which appear directly related to the stated objective of resolving issue #57.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@supervoidcoder supervoidcoder marked this pull request as ready for review January 29, 2026 21:46
@supervoidcoder
Copy link
Owner Author

@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)

@coderabbitai coderabbitai bot added enhancement New feature or request priority: medium Medium priority ready-for-review Ready for review windows Windows-specific issues labels Jan 29, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 29, 2026

@supervoidcoder That's awesome! 4 commits for a complete feature is pretty impressive – I love the efficiency! 🎉

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
Learnt from: supervoidcoder
Repo: supervoidcoder/win-witr PR: 48
File: README.md:38-38
Timestamp: 2026-01-21T01:52:33.870Z
Learning: For supervoidcoder's README files in the win-witr repository: The user prefers an intentionally informal, humorous tone with casual language. Grammar formality suggestions (like "anyways" vs "anyway") should be skipped as they don't align with the intentional style of the documentation.
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

@supervoidcoder
Copy link
Owner Author

@coderabbitai welp I did speak too soon lol

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 29, 2026

Tip

For best results, initiate chat on the files or code changes.

@supervoidcoder Haha, happens to the best of us! 😄 Iterating is just part of the process – at least you caught it early. No worries at all, we'll get it sorted!

The important thing is that you're making solid progress on the feature. Keep it up! 🚀


🧠 Learnings used
Learnt from: supervoidcoder
Repo: supervoidcoder/win-witr PR: 48
File: README.md:38-38
Timestamp: 2026-01-21T01:52:33.870Z
Learning: For supervoidcoder's README files in the win-witr repository: The user prefers an intentionally informal, humorous tone with casual language. Grammar formality suggestions (like "anyways" vs "anyway") should be skipped as they don't align with the intentional style of the documentation.

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
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 GetProcessNameFromPid is clean, and the N/A fallback for missing processes is handled correctly in both VT100 and non-VT100 paths.

Optional nits:

  • Prefer procName.empty() over procName == "" for idiomatic C++
  • Line 592 has a redundant \033[0m after procName (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;

@supervoidcoder supervoidcoder merged commit ca7015c into main Jan 29, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request priority: medium Medium priority ready-for-review Ready for review windows Windows-specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

target entry

1 participant