Chore: Optimizations

[dayvster]

This pull request introduces several improvements to the development workflow, build configuration, and project documentation. The most significant changes are the addition of a commit message linting workflow, enhancements to the CMake build system (including new options for tests and warnings), and updates to the project's contribution guidelines to clarify commit message conventions.

Development workflow improvements:

• Added a new GitHub Actions workflow (.github/workflows/commitlint.yml) to enforce Conventional Commits for all pushes and pull requests targeting main and dev branches. This workflow checks commit messages for compliance, supporting both standard and project-specific types.

Build system enhancements (CMake):

• Enabled export of compile commands with CMAKE_EXPORT_COMPILE_COMMANDS for better tooling support (e.g., clang-tidy, language servers).
• Introduced CMake options: BUILD_TESTS (to toggle building of unit tests) and ENABLE_STRICT_WARNINGS (to enable strict compiler warnings for developer builds).
• Improved source file collection logic to only include implementation sources from src/ and explicitly add main.cpp if present, reducing accidental inclusion of unrelated files.
• Added logic to build and register unit tests (menu_parser_test, discovery_test) when BUILD_TESTS is enabled, and enabled strict warning flags when ENABLE_STRICT_WARNINGS is set.

Documentation and conventions:

• Updated CONTRIBUTING.md to clarify accepted commit message types, including the addition of bugfix: (as an alias for fix:) and type: for project-specific metadata changes. The preferred types are now explicitly listed.

Other:

• Added a new debug output file dump2.txt containing application discovery results, likely for development or testing purposes.

[Copilot]

Pull request overview

This pull request introduces significant improvements to the build system, codebase architecture, and development workflow for the DLauncher application launcher. The changes focus on performance optimizations, better XDG standards compliance, improved testability, and enhanced developer tooling.

Changes:

• Added commitlint GitHub Actions workflow to enforce Conventional Commits standards
• Enhanced CMake build system with compile commands export, test building options, and strict warning flags
• Refactored application discovery with caching, precomputed search fields, string interning, and configurable scan paths

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
.github/workflows/commitlint.yml	New workflow for commit message validation
CMakeLists.txt	Added build options (BUILD_TESTS, ENABLE_STRICT_WARNINGS), compile commands export, and test targets
CONTRIBUTING.md	Updated with commit message type aliases and conventions
.gitignore	Added Qt auto-gen artifacts directory
src/core/xdg.{h,cpp}	New XDG Base Directory specification helpers
src/core/intern.{h,cpp}	New string interning module for memory optimization
src/core/frequency_store.{h,cpp}	New frequency tracking abstraction
src/core/apps/readApps.{h,cpp}	Major refactor with caching, precomputed fields, system app filtering, and dump diagnostics
src/utils/json.hpp	Enhanced directory creation for save operations
src/ui/components/list.cpp	Added debug logging for UI diagnostics
main.cpp	Integrated XDG paths, FrequencyStore, and new CLI flags (--dump, --show-system)
test/discovery_test.cpp	New integration test for app discovery
src/components/appRow/appRow.h	Updated include path to new location
dump*.txt	Development diagnostic files
src/{apps,utils}/readApps.{h,cpp}	Removed old file locations

Comments suppressed due to low confidence (5)

src/components/appRow/appRow.h:5

• The include path "core/apps/readApps.h" should be "src/core/apps/readApps.h" for consistency with the project's include path conventions and the CMake configuration which adds CMAKE_SOURCE_DIR to the include directories.
  src/core/apps/readApps.cpp:175
• The DumpAndPrint function doesn't use the configurable desktopAppPaths member variable. Instead, it hardcodes a priorityDirs list (lines 172-175), which means calls to SetDesktopAppPaths won't affect this function's behavior. This inconsistency could lead to confusing results where LoadApps and DumpAndPrint scan different directories. Consider using the same path expansion logic from LoadApps (lines 39-55) to respect the configured paths.

void AppReader::DumpAndPrint(bool includeHidden, bool showSystem)
{
  const char* home = getenv("HOME");
  std::string homeStr = home ? std::string(home) : std::string();
  std::vector<std::string> priorityDirs = {
      homeStr + "/.local/share/applications",
      "/usr/local/share/applications",
      "/usr/share/applications"};

src/core/apps/readApps.cpp:329

• ReadDesktopApps and SearchApps should leverage the precomputed name_lc and exec_lc fields from DesktopApp. Currently, ReadDesktopApps calls toLower(app.name) on line 301, and SearchApps calls toLower(app.name) and toLower(app.exec) on lines 328 and 330. These fields are already computed during LoadApps and stored in app.name_lc and app.exec_lc, so reusing them would improve performance.

  for (const auto &app : allApps)
  {
    if (searchTerm.empty() ||
        toLower(app.name).find(searchLower) != std::string::npos)
    {
      filtered.push_back(app);
    }
  }

  if (limit > 0 && filtered.size() > static_cast<size_t>(limit))
  {
    filtered.resize(limit);
  }

  return filtered;
}

const std::vector<DesktopApp> &AppReader::GetAllApps() const { return allApps; }

std::vector<DesktopApp> AppReader::SearchApps(std::string searchTerm, int limit,
                                              bool isFuzzy)
{
  std::vector<DesktopApp> results;
  std::string searchLower = toLower(searchTerm);

  if (isFuzzy)
  {
    std::set<std::pair<std::string, std::string>> seen;
    for (const auto &app : allApps)
    {
      std::string appNameLower = toLower(app.name);
      appNameLower.erase(remove_if(appNameLower.begin(), appNameLower.end(), ::isspace), appNameLower.end());

CONTRIBUTING.md:37

• The description for type: on line 37 is unclear. It states "small project-specific type to indicate the change relates to 'type' metadata" but doesn't explain what "type metadata" means in the context of this project. Consider providing a clearer example or explanation, such as "type system changes" or "TypeScript type definitions" if that's what's meant, or clarify what project-specific metadata this refers to.
  src/utils/json.hpp:105
• Redundant create_directories call on line 105. The parent directory was already created on line 89, so this call is unnecessary when the rename fails. This doesn't cause functional issues but is wasteful.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The cachePath() and ensureCacheDir() functions duplicate logic from the xdg module (src/core/xdg.h/cpp). Consider using xdg::cachePath("dlauncher/apps.cache") and xdg::ensureDir() instead of maintaining duplicate implementations. This would improve maintainability and ensure consistent behavior across the application.

[Copilot]

Duplicate #include <fstream> directive: the header is included on both lines 6 and 11. Remove one of these duplicates.

Suggested change

#include <fstream>

[Copilot]

The comment on line 50 is misleading or incorrect. It states "By default include all .desktop entries (don't skip NoDisplay/Hidden/OnlyShowIn)" but includeHidden is initialized to false on line 51, which means NoDisplay and Hidden entries WILL be skipped by default. Update the comment to accurately reflect the default behavior.

Suggested change

	// By default include all .desktop entries (don't skip NoDisplay/Hidden/OnlyShowIn)
	// By default skip NoDisplay/Hidden/OnlyShowIn .desktop entries; use --include-hidden/--show-hidden to include them

[Copilot]

The logic for handling commit ranges on lines 47-52 has a flaw. When RANGE is a single SHA (line 47), git rev-parse will verify that SHA exists and git rev-list will list just that one commit. However, when RANGE is something like "origin/main..HEAD" (line 26 or 35), git rev-parse --verify will fail because that's a range notation, not a single ref. This means the fallback on lines 50-51 will always trigger for push/PR events, checking the last 100 commits instead of just the new commits in the range. Use git rev-list directly without the rev-parse check, or use "git rev-list $RANGE 2>/dev/null || git rev-list HEAD -n 100".

Suggested change

	# gather commits; if RANGE is a single sha, handle that
	if git rev-parse --verify "$RANGE" >/dev/null 2>&1; then
	commits=$(git rev-list --no-merges $RANGE)
	else
	# fallback: recent 100 commits
	commits=$(git rev-list --no-merges HEAD -n 100)
	fi
	# gather commits; try the specified range, fall back to recent 100 commits if it is invalid
	commits=$(git rev-list --no-merges "$RANGE" 2>/dev/null || git rev-list --no-merges HEAD -n 100)

[Copilot]

The include path "core/intern.h" should be "../core/intern.h" to be consistent with other includes in this file (lines 2 and 10 use "../" prefix). This inconsistency may cause build failures depending on the include path configuration.

Suggested change

	#include "core/intern.h"
	#include "../core/intern.h"

[Copilot]

The test doesn't handle potential cleanup failures. If the test fails before reaching lines 34-35, the temporary directory and file will not be cleaned up. Consider using RAII or a try-catch-finally pattern to ensure cleanup happens even if the test fails. Alternatively, use std::filesystem::remove_all to recursively remove the directory and its contents in one call.

[Copilot]

The discovery_test executable (line 80) links against Qt6::Widgets but the test code doesn't use any Qt functionality - it only uses standard C++ libraries (filesystem, fstream, cassert) and the core app reader classes. This unnecessary dependency increases build time and binary size. Remove Qt6::Widgets from the target_link_libraries for discovery_test unless Qt functionality is actually required.

Suggested change

target_link_libraries(discovery_test PRIVATE Qt6::Widgets)

[Copilot]

Duplicate include directive: "list.h" is included twice on lines 1 and 3. Remove one of these duplicate includes.

Suggested change

#include "list.h"

[Copilot]

The target_include_directories has a duplicate entry: "${CMAKE_SOURCE_DIR}/src/core" appears on both line 65 and line 69. Remove the duplicate entry.

Suggested change

${CMAKE_SOURCE_DIR}/src/core

[Copilot]

The DumpAndPrint logic for skipping based on OnlyShowIn and NotShowIn (lines 202-211) doesn't match the LoadApps logic. In LoadApps, these fields are parsed but never actually used to filter apps. Either LoadApps should also filter based on these fields, or the DumpAndPrint function shouldn't show them as skip reasons. This inconsistency means DumpAndPrint will report entries as "SKIPPED" that LoadApps would actually include.

Suggested change

	else if (!includeHidden && !app.onlyShowIn.empty())
	{
	skipped = true;
	reason = "OnlyShowIn";
	}
	else if (!includeHidden && !app.notShowIn.empty())
	{
	skipped = true;
	reason = "NotShowIn";
	}