Skip to content

Conversation

@CarterLi
Copy link
Member

No description provided.

@CarterLi CarterLi requested a review from Copilot July 18, 2025 09:45
Copilot

This comment was marked as outdated.

@CarterLi CarterLi requested a review from Copilot July 18, 2025 10:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This is a release PR for version 2.48.1 that introduces conditional module support, improves package manager handling, and fixes cross-platform compatibility issues. The changes include:

  • Adding conditional module execution based on system and architecture
  • Adding a combined option for package managers to merge related counts
  • Improving process spawning reliability on Windows and fixing compatibility for Android/OpenBSD
  • Adding Openbox WM version detection support

Reviewed Changes

Copilot reviewed 89 out of 89 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Multiple module files Add support for "condition" key in JSON parsing to enable conditional modules
src/modules/packages/* Add combined package manager option and related logic
src/common/jsonconfig.c Implement conditional module execution logic based on system/arch matching
src/common/processing_*.c Improve process spawning and fix platform compatibility issues
src/common/io/* Add ffGetNullFD function for consistent null file descriptor handling
src/detection/wm/wm_linux.c Add Openbox version detection and refactor common version extraction
Version/config files Update version numbers, changelog, and configuration schemas
Comments suppressed due to low confidence (1)

src/detection/wm/wm_linux.c:179

  • [nitpick] The function name 'extractCommonWmVersion' could be more descriptive. Consider 'extractVersionFromBinary' or 'extractVersionPattern' to better indicate its purpose of extracting version strings matching x.y.z pattern.
static bool extractCommonWmVersion(const char* line, FF_MAYBE_UNUSED uint32_t len, void *userdata)

Comment on lines +103 to +104
FF_PRINT_PACKAGE_NAME(brew, "brew")
FF_PRINT_PACKAGE_NAME(brewCask, "brew-cask")
Copy link

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

The macro call is missing a closing brace according to the new macro definition pattern. The FF_PRINT_PACKAGE_NAME macro now uses curly braces but this call doesn't follow the pattern.

Suggested change
FF_PRINT_PACKAGE_NAME(brew, "brew")
FF_PRINT_PACKAGE_NAME(brewCask, "brew-cask")
FF_PRINT_PACKAGE_NAME({brew, "brew"})
FF_PRINT_PACKAGE_NAME({brewCask, "brew-cask"})

Copilot uses AI. Check for mistakes.

return "CreateFileW(L\"\\\\.\\pipe\\FASTFETCH-$(PID)\") failed";

PROCESS_INFORMATION piProcInfo = {0};
PROCESS_INFORMATION piProcInfo = {};
Copy link

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

[nitpick] Using empty brace initialization '{}' instead of '{0}' is a C++ style. In C, '{0}' is more conventional and explicitly shows zero-initialization intent.

Suggested change
PROCESS_INFORMATION piProcInfo = {};
PROCESS_INFORMATION piProcInfo = {0};

Copilot uses AI. Check for mistakes.

@CarterLi CarterLi merged commit 02aafbd into master Jul 18, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant