Skip to content

feat(tldr.py): enhance TLDR command with caching, platform detection,… #859

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

Merged
merged 9 commits into from
Jun 2, 2025

Conversation

kzndotsh
Copy link
Contributor

@kzndotsh kzndotsh commented May 13, 2025

… and improved Discord integration

Introduce a comprehensive TLDR client to manage fetching, caching, and displaying TLDR pages. This includes support for multiple languages and platforms, with automatic cache updates and platform detection. The Discord integration is improved with pagination for long pages and autocomplete for command and platform inputs. These changes aim to enhance user experience by providing faster access to TLDR pages, supporting more platforms and languages, and offering a more interactive Discord interface.

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. If this change fixes any issues please put "Fixes #XX" in the description. Please also ensure to add the appropriate labels to the PR.

Guidelines

  • My code follows the style guidelines of this project (formatted with Ruff)

  • I have performed a self-review of my own code

  • I have commented my code, particularly in hard-to-understand areas

  • I have made corresponding changes to the documentation if needed

  • My changes generate no new warnings

  • I have tested this change

  • Any dependent changes have been merged and published in downstream modules

  • I have added all appropriate labels to this PR

  • I have followed all of these guidelines.

How Has This Been Tested? (if applicable)

Please describe how you tested your code. e.g describe what commands you ran, what arguments, and any config stuff (if applicable)

Screenshots (if applicable)

Please add screenshots to help explain your changes.

Additional Information

Please add any other information that is important to this PR.

Summary by Sourcery

Implement a robust hot reload subsystem and a full-featured TLDR client integration for the Discord bot. The hot reload system now offers configurable dependency tracking, debouncing, syntax validation, and observability. The new TldrClient wrapper handles caching, platform and language priorities, and is exposed through a Discord Cog with autocompletion and pagination for an enhanced user experience.

New Features:

  • Add comprehensive hot reload system with file watching, dependency tracking, debouncing, hot patching, performance monitoring, and Sentry integration
  • Introduce a standalone TldrClient for fetching, caching, and normalizing TLDR pages with platform detection and language fallbacks
  • Add Discord Cog for TLDR command with slash/prefix support, autocomplete for command/platform/language inputs, and pagination for long pages

Enhancements:

  • Refactor command usage generation and flag utilities into reusable functions and update existing cogs to use them
  • Improve bot setup to load cogs, initialize hot reload, set default presence on ready, and streamline extension loading
  • Overhaul Dockerfile to create and permission TLDR cache directory and remove tealdeer dependency

CI:

  • Bump gitleaks hook to v8.27.0 in pre-commit config

… and improved Discord integration

Introduce a comprehensive TLDR client to manage fetching, caching, and
displaying TLDR pages. This includes support for multiple languages and
platforms, with automatic cache updates and platform detection. The
Discord integration is improved with pagination for long pages and
autocomplete for command and platform inputs. These changes aim to
enhance user experience by providing faster access to TLDR pages,
supporting more platforms and languages, and offering a more interactive
Discord interface.
@kzndotsh kzndotsh linked an issue May 13, 2025 that may be closed by this pull request
Copy link
Contributor

sourcery-ai bot commented May 13, 2025

Reviewer's Guide

This PR overhauls the bot’s hot-reload framework for smarter dependency tracking, error handling, and debounced reloads, and introduces a full-featured TLDR page client with local caching, multi-platform and multi-language support, plus a new Discord cog that provides slash/prefix commands, autocomplete, and paginated embeds for TLDR entries.

Sequence Diagram for TLDR Slash Command Execution

sequenceDiagram
    actor User
    participant Discord
    participant TuxBot
    participant TldrCog
    participant TldrClient
    participant LocalCache
    participant RemoteTldrRepo
    participant TldrPaginatorView

    User->>Discord: /tldr command="tar", platform="linux"
    Discord->>TuxBot: InteractionCreate (command="tar", platform="linux")
    TuxBot->>TldrCog: slash_tldr(interaction, command="tar", platform="linux")
    TldrCog->>TldrClient: fetch_tldr_page("tar", ["en"], "linux")
    TldrClient->>LocalCache: Check for "tar" on "linux" in "en"
    alt Cache Miss
        LocalCache-->>TldrClient: Not found or stale
        TldrClient->>RemoteTldrRepo: Request "tar" page (linux, en)
        RemoteTldrRepo-->>TldrClient: Page Content Markdown
        TldrClient->>LocalCache: Store Page Content
    else Cache Hit
        LocalCache-->>TldrClient: Page Content Markdown
    end
    TldrClient-->>TldrCog: (Page Content, "linux")
    TldrCog->>TldrClient: format_tldr_for_discord(Page Content, ...)
    TldrClient-->>TldrCog: Formatted Description (potentially long)
    TldrCog->>TldrClient: split_long_text(Formatted Description)
    TldrClient-->>TldrCog: [page1_content, page2_content]
    TldrCog->>TldrPaginatorView: new TldrPaginatorView([page1, page2], title, user, bot)
    TldrPaginatorView-->>TldrCog: view_instance
    TldrCog->>Discord: Send embed with page1_content and view_instance
    Discord-->>User: Display TLDR page with pagination buttons

    User->>Discord: Click "Next" button
    Discord->>TuxBot: InteractionCreate (button_click)
    TuxBot->>TldrPaginatorView: view_instance.next()
    TldrPaginatorView->>Discord: Edit message with page2_content
    Discord-->>User: Display next page
Loading

Sequence Diagram for Hot Reload File Change Event

sequenceDiagram
    participant FileSystem
    participant WatchdogObserver
    participant CogWatcher
    participant DependencyGraph
    participant FileHashTracker
    participant TuxBot

    Note over FileSystem, TuxBot: Developer modifies a Python file
    FileSystem->>WatchdogObserver: FileModifiedEvent(path)
    WatchdogObserver->>CogWatcher: on_modified(event)
    CogWatcher->>CogWatcher: Debounce delay
    CogWatcher->>CogWatcher: _handle_file_change_debounced(path)
    CogWatcher->>DependencyGraph: has_file_changed(path)
    DependencyGraph->>FileHashTracker: get_file_hash(path)
    FileHashTracker-->>DependencyGraph: current_hash
    DependencyGraph-->>CogWatcher: bool (changed or not)
    alt File Content Changed
        CogWatcher->>CogWatcher: validate_python_syntax(path)
        alt Syntax Valid
            CogWatcher-->>CogWatcher: True
            CogWatcher->>DependencyGraph: update_dependencies(path, module_name)
            Note over DependencyGraph: Scans imports, updates graph
            CogWatcher->>DependencyGraph: get_transitive_dependents(module_name)
            DependencyGraph-->>CogWatcher: [dependent_ext1, ...]
            loop For each dependent extension
                CogWatcher->>TuxBot: Schedule _async_reload_extension(dependent_ext1)
                TuxBot->>TuxBot: bot.reload_extension(dependent_ext1)
            end
        else Syntax Invalid
            CogWatcher-->>CogWatcher: False (logs error, skips reload)
        end
    else File Content Not Changed
        DependencyGraph-->>CogWatcher: False (skips reload)
    end
Loading

Class Diagram for TLDR System Components

classDiagram
    class TldrClient {
        <<static>> normalize_page_name(name str) str
        <<static>> get_cache_file_path(command str, platform str, language str) Path
        <<static>> have_recent_cache(command str, platform str, language str) bool
        <<static>> load_page_from_cache(command str, platform str, language str) str
        <<static>> store_page_to_cache(page str, command str, platform str, language str) void
        <<static>> detect_platform() str
        <<static>> get_language_priority(user_language str) list~str~
        <<static>> get_platform_priority(user_platform_input str) list~str~
        <<static>> fetch_tldr_page(command str, languages list~str~, platform_preference str) tuple~str,str~
        <<static>> list_tldr_commands(language str, platform_filter str) list~str~
        <<static>> parse_placeholders(line str, show_short bool, show_long bool, show_both bool, highlight bool) str
        <<static>> format_tldr_for_discord(md str, show_short bool, show_long bool, show_both bool) str
        <<static>> update_tldr_cache(language str) str
        <<static>> cache_needs_update(language str) bool
        <<static>> split_long_text(text str, max_len int) list~str~
    }

    class TldrCog {
      -bot: Tux
      -default_language: str
      -prefix_tldr: commands.Command
      +cog_load()
      +command_autocomplete(interaction, current) list~app_commands.Choice~
      +platform_autocomplete(interaction, current) list~app_commands.Choice~
      +language_autocomplete(interaction, current) list~app_commands.Choice~
      +slash_tldr(interaction, command str, platform str, language str, show_short bool, show_long bool, show_both bool) None
      +prefix_tldr(ctx, command str, flags TldrFlags) None
      -_handle_tldr_command_slash(...)
      -_handle_tldr_command_prefix(...)
    }

    class TldrFlags {
      +platform: str | None
      +language: str | None
      +show_short: bool
      +show_long: bool
      +show_both: bool
    }

    class TldrPaginatorView {
      -pages: list~str~
      -page: int
      -title: str
      -user: discord.User
      -bot: Tux
      +prev(interaction, button)
      +next(interaction, button)
      +update_message(interaction)
    }

    TldrCog ..> TldrClient : Uses
    TldrCog ..> TldrFlags : Uses for prefix command
    TldrCog ..> TldrPaginatorView : Creates & Uses
Loading

Class Diagram for Enhanced Hot Reload System

classDiagram
    class HotReloadConfig {
        +debounce_delay: float
        +cleanup_threshold: int
        +max_dependency_depth: int
        +enable_hot_patching: bool
        +enable_dependency_tracking: bool
        +validate_syntax: bool
        +watch_patterns: Sequence~str~
        +ignore_patterns: Sequence~str~
    }

    class HotReloadError {
        <<Abstract>>
        +message: str
        +context: dict
    }
    DependencyResolutionError --|> HotReloadError
    FileWatchError --|> HotReloadError
    ModuleReloadError --|> HotReloadError
    ConfigurationError --|> HotReloadError

    class FileHashTracker {
        -_file_hashes: dict~str, str~
        +get_file_hash(file_path Path) str
        +has_file_changed(file_path Path) bool
        +clear_cache() void
    }

    class ClassDefinitionTracker {
        -_class_registry: dict
        +scan_class_definitions(file_path Path, module_name str) dict
        +register_classes(module_name str, file_path Path) void
        +get_changed_classes(module_name str, file_path Path) list~str~
        +clear_cache() void
    }

    class DependencyGraph {
        <<Interface>> DependencyTracker
        -_config: HotReloadConfig
        -_module_dependencies: dict~str, set~str~~~
        -_reverse_dependencies: dict~str, set~str~~~
        -_file_tracker: FileHashTracker
        -_class_tracker: ClassDefinitionTracker
        +scan_dependencies(file_path Path) set~str~
        +update_dependencies(file_path Path, module_name str) void
        +get_dependents(module_name str) set~str~
        +get_transitive_dependents(module_name str) set~str~
        +has_file_changed(file_path Path) bool
        +register_classes(module_name str, file_path Path) void
        +get_changed_classes(module_name str, file_path Path) list~str~
    }

    class CogWatcher {
        -bot: BotProtocol
        -path: str
        -loop: asyncio.AbstractEventLoop
        -dependency_graph: DependencyGraph
        -_config: HotReloadConfig
        -_debounce_timers: dict~str, asyncio.Handle~
        +on_modified(event watchdog.events.FileSystemEvent) void
        +_handle_file_change_debounced(file_path Path) void
        +_reload_extension(extension str) void
        +_async_reload_extension(extension str) void
        +start() void
        +stop() void
    }

    CogWatcher o-- HotReloadConfig
    CogWatcher o-- DependencyGraph
    DependencyGraph o-- HotReloadConfig
    DependencyGraph o-- FileHashTracker
    DependencyGraph o-- ClassDefinitionTracker
    class HotReload {
        <<Cog>>
        -bot: commands.Bot
        -watcher: CogWatcher
        +cog_unload()
    }
    HotReload o-- CogWatcher
Loading

File-Level Changes

Change Details Files
Enhanced hot-reload system with dependency graph, file-hash tracking, and debounced reloads
  • Replace simple watcher with CogWatcher using Watchdog observer
  • Add HotReloadConfig, protocols, and rich span-tagged observability
  • Implement FileHashTracker, ClassDefinitionTracker, and DependencyGraph
  • Introduce debounce timers, syntax validation, hot-patching, and robust error handling
tux/utils/hot_reload.py
tux/bot.py
Refactor utility modules: move flag and usage logic to functions module
  • Remove usage-generation helpers from flags.py
  • Implement is_optional_param, get_matching_string, and generate_usage in functions.py
  • Introduce TldrFlags class for TLDR command options
tux/utils/flags.py
tux/utils/functions.py
Add TldrClient to fetch, cache, and format TLDR pages
  • Normalize page names and compute cache paths
  • Detect platform, prioritize languages, and manage cache age
  • Fetch remote markdown, store locally, and fallback per spec
  • Parse TLDR placeholders and format output for Discord
tux/wrappers/tldr.py
Introduce Discord TLDR cog with slash/prefix commands and autocomplete
  • Create slash command with platform/language autocompletion
  • Implement prefix command using TldrFlags for flags
  • Integrate TldrClient fetch/format logic into command handlers
  • Handle long pages via paginated view support
tux/cogs/tools/tldr.py
Add TldrPaginatorView for interactive page navigation
  • Define Previous/Next buttons with interaction checks
  • Implement on_timeout to clear view and update_message to refresh embeds
tux/ui/views/tldr.py
Update Dockerfile to prepare TLDR cache directory
  • Create and chown .cache/tldr for nonroot user
  • Remove automatic tealdeer installation/update
Dockerfile

Possibly linked issues

  • #1337: PR implements TLDR pagination to condense long pages, addressing the issue's vertical space problem.
  • #1: PR implements improved tldr placeholder parsing and display options, fixing cluttered output as requested.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

cloudflare-workers-and-pages bot commented May 13, 2025

Deploying tux with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1782eb1
Status: ✅  Deploy successful!
Preview URL: https://c6ab3fe8.tux-afh.pages.dev
Branch Preview URL: https://tldr-rewrite.tux-afh.pages.dev

View logs

kzndotsh added 8 commits May 30, 2025 02:42
Extract TLDR client logic into a separate module to improve code organization and maintainability. This change separates the core TLDR functionality from Discord-specific UI components, allowing for easier testing and potential reuse in non-Discord contexts. The refactor also includes improvements to caching logic, platform detection, and language handling, enhancing the overall robustness and flexibility of the TLDR integration.
…up process

Remove the unused `cog_watcher` attribute from the `Tux` class to clean up the code. This attribute was not being utilized effectively and its removal simplifies the class structure. Additionally, improve the setup process by ensuring that cogs and hot reload systems are initialized in a more structured manner, enhancing maintainability and readability.

fix(cogs): update import paths for generate_usage function

Change import paths for `generate_usage` from `tux.utils.flags` to `tux.utils.functions` across multiple cog files. This change reflects the relocation of the `generate_usage` function to a more appropriate module, improving code organization and clarity.

feat(bot.py): add on_ready event handler to set bot status

Introduce an `on_ready` event handler in the `Tux` class to set the bot's status to "watching for /help" when the bot is ready. This provides users with a clear indication of the bot's availability and primary command.

refactor(hot_reload.py): enhance hot reload system with dependency tracking and error handling

Significantly enhance the hot reload system by introducing smart dependency tracking, improved error handling, and performance monitoring. This includes the addition of a `DependencyGraph` class for managing module dependencies, a `FileHashTracker` for change detection, and a `ClassDefinitionTracker` for hot patching capabilities. These improvements make the hot reload system more robust and efficient, allowing for better management of code changes and reducing the risk of errors during reloads.
The command parameter is now explicitly separated from flags,
improving readability and understanding of the function's
parameters. This change also includes a more descriptive
docstring for the command parameter.

fix(flags.py): remove command flag from TldrFlags and add aliases

The command flag is removed from TldrFlags as it is now a
separate parameter in the prefix_tldr function. Aliases are
added to other flags for better usability and flexibility.

feat(hot_reload.py): add syntax validation before hot reload

Introduces a function to validate Python syntax before
attempting a hot reload, preventing unnecessary reloads and
Sentry spam due to syntax errors. Also, adds a mechanism to
filter out common development errors from being sent to Sentry,
reducing noise in error tracking.

These changes improve code clarity, usability, and robustness
by ensuring that only valid Python files are reloaded and
reducing unnecessary error reporting.
Update the gitleaks pre-commit hook to the latest version to ensure
the latest security checks and improvements are included. This helps
maintain the security and integrity of the codebase by using the most
up-to-date version of the tool.
…ling

Remove the tealdeer package from the Dockerfile as it is no longer
required. Instead, create a TLDR cache directory with appropriate
permissions for the nonroot user. This change ensures that the
application image is leaner by removing unnecessary packages and
improves security by setting up the cache directory with the correct
permissions for the nonroot user.
Introduce a new Discord cog for TLDR command integration,
enabling users to fetch TLDR pages for CLI commands directly
within Discord. This feature supports both slash and prefix
commands, providing flexibility in usage. It includes
autocomplete for command, platform, and language parameters,
enhancing user experience by making it easier to find the
desired TLDR pages. The cog also manages cache updates to
ensure users receive the most recent information. This
enhancement aims to improve the bot's utility by offering
quick access to command usage summaries, which is especially
useful for users who frequently work with command-line tools.
@kzndotsh kzndotsh marked this pull request as ready for review June 2, 2025 08:53
@kzndotsh kzndotsh merged commit 9defdd1 into main Jun 2, 2025
8 checks passed
@kzndotsh kzndotsh deleted the tldr-rewrite branch June 2, 2025 08:54
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @kzndotsh - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • time.sleep() call; did you mean to leave this in? (link)
  • Blocking time.sleep in event loop callback (link)

General comments:

  • Refactor the nearly identical logic in _handle_tldr_command_slash and _handle_tldr_command_prefix into a shared helper to reduce duplication and ease maintenance.
  • Consider converting TldrClient.fetch_tldr_page (and other I/O methods) to fully async (e.g. using aiohttp) instead of delegating to run_in_executor, to avoid blocking threads and simplify error handling.
  • Filesystem scans in TldrClient.list_tldr_commands run on every autocomplete request—introducing a cached index or pre‐loading the command list at startup would greatly improve autocomplete responsiveness.
Here's what I looked at during the review
  • 🔴 General issues: 1 blocking issue, 7 other issues
  • 🔴 Security: 1 blocking issue
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@@ -35,151 +161,710 @@ def path_from_extension(extension: str) -> Path:


def get_extension_from_path(file_path: Path, base_dir: Path) -> str | None:
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Special-case handling for init.py was removed

Consider adding logic to remove the __init__ suffix so that package directories are mapped correctly, as in the previous implementation.

else:
return True

except SyntaxError as e:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Nested exception handlers overlap and some are unreachable

The inner try block catches all SyntaxError exceptions, making the outer handler redundant. Refactor to catch SyntaxError only around ast.parse, and use a single except OSError for file operations.

The extension to reload.
"""

"""Reload an extension with proper error handling."""
try:
# Add a small delay to ensure file write is complete
time.sleep(0.1)
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Blocking time.sleep in event loop callback

Replace time.sleep(0.1) with await asyncio.sleep(0.1) to prevent blocking the event loop.

logger.warning(f"Could not find file for extension {extension}, expected at {path}")
# Pre-populate hash cache for all Python files in watched directories
# This eliminates "first encounter" issues for any file
cached_files = self._populate_all_file_hashes()
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (performance): Eager full-tree hashing may impact startup performance

Pre-populating hashes for all Python files can be costly in large repositories. Consider restricting this to specific file types or making it configurable to avoid unnecessary overhead.

Suggested implementation:

        # Optionally pre-populate hash cache for files in watched directories
        # This can be disabled or restricted via configuration to avoid startup overhead
        prepopulate_hashes = getattr(self, "prepopulate_hashes", True)
        hash_extensions = getattr(self, "hash_extensions", [".py"])

        cached_files = 0
        if prepopulate_hashes:
            cached_files = self._populate_all_file_hashes(hash_extensions)
            if cached_files > 0:
                logger.debug(f"Pre-populated hash cache for {cached_files} files with extensions: {hash_extensions}")
    def _populate_all_file_hashes(self, extensions=None) -> int:
        """
        Pre-populate hash cache for all files in watched directories matching given extensions.
        :param extensions: List of file extensions to hash (e.g., [".py"])
        """
        if extensions is None:
            extensions = [".py"]
        cached_count = 0

        # Get the root watch path (this includes the entire tux directory)
        watch_root = Path(self.path)
        for ext in extensions:
            for file_path in watch_root.rglob(f"*{ext}"):
                try:
                    self._get_file_hash(file_path)
                    cached_count += 1
                except Exception as e:
                    logger.warning(f"Failed to hash {file_path}: {e}")
  • You should add prepopulate_hashes and hash_extensions as attributes to your class, or pass them in via constructor/configuration, to allow users to control this behavior.
  • Update any documentation or configuration files to reflect the new options.

return self._class_tracker.get_changed_classes(module_name, file_path)
return []

def _resolve_relative_import(self, file_path: Path, module: str | None, level: int) -> str | None:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Pure relative imports (from . import x) are ignored

Currently, cases where node.module is None (such as from . import foo) are not handled. Consider treating module=None as a local package import to support these scenarios.

Suggested implementation:

    def _resolve_relative_import(self, file_path: Path, module: str | None, level: int, imported_name: str | None = None) -> str | None:
        """Resolve relative imports to absolute module names.

        If `module` is None (pure relative import), treat as importing from the current package.
        """
        try:
            relative_path = file_path.relative_to(base_dir)
            # Remove the .py extension
            path_without_ext = relative_path.with_suffix("")
            # Convert to dot notation
            extension = str(path_without_ext).replace(os.sep, ".")
            if module is None and imported_name is not None:
                # Pure relative import: from . import foo
                # Remove the last component (the module itself) to get the package
                package = ".".join(extension.split(".")[:-1])
                if package:
                    return f"{package}.{imported_name}"
                else:
                    return imported_name
        except ValueError:
            return None
        else:
  • You will need to ensure that when calling _resolve_relative_import, you pass the imported_name argument (e.g., the name being imported in from . import foo).
  • If the code that calls _resolve_relative_import does not have access to the imported name, you may need to update the call sites to provide it.

Comment on lines +784 to +788
try:
self.observer.stop()
self.observer.join(timeout=5.0) # Add timeout to prevent hanging
except Exception as e:
logger.error(f"Error stopping file watcher: {e}")
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (bug_risk): Observer join timeout not checked or reported

Consider logging a warning or raising an error if the observer thread does not terminate within the timeout period.

Suggested change
try:
self.observer.stop()
self.observer.join(timeout=5.0) # Add timeout to prevent hanging
except Exception as e:
logger.error(f"Error stopping file watcher: {e}")
try:
self.observer.stop()
self.observer.join(timeout=5.0) # Add timeout to prevent hanging
if self.observer.is_alive():
logger.warning("File watcher observer thread did not terminate within the timeout period.")
except Exception as e:
logger.error(f"Error stopping file watcher: {e}")

Comment on lines +1065 to +1069
# Clear related module cache entries before reloading
if modules_to_clear := [key for key in sys.modules if key.startswith(extension)]:
logger.debug(f"Clearing {len(modules_to_clear)} cached modules for {extension}")
for module_key in modules_to_clear:
del sys.modules[module_key]
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (bug_risk): Clearing all sys.modules entries by prefix may overreach

Consider limiting module removal to those directly imported by the cog, or explicitly tracking relevant module names, to avoid affecting unrelated subpackages.

Suggested change
# Clear related module cache entries before reloading
if modules_to_clear := [key for key in sys.modules if key.startswith(extension)]:
logger.debug(f"Clearing {len(modules_to_clear)} cached modules for {extension}")
for module_key in modules_to_clear:
del sys.modules[module_key]
# Clear only the extension module and its direct submodules that are part of the same package directory
import importlib.util
import os
module = sys.modules.get(extension)
if module and hasattr(module, "__file__") and module.__file__:
extension_root = os.path.dirname(os.path.abspath(module.__file__))
modules_to_clear = []
for key, mod in list(sys.modules.items()):
if key == extension or key.startswith(f"{extension}."):
mod_file = getattr(mod, "__file__", None)
if mod_file and os.path.abspath(os.path.dirname(mod_file)).startswith(extension_root):
modules_to_clear.append(key)
if modules_to_clear:
logger.debug(f"Clearing {len(modules_to_clear)} cached modules for {extension}: {modules_to_clear}")
for module_key in modules_to_clear:
del sys.modules[module_key]

error_msg = str(exception).lower()

# Common development errors that shouldn't spam Sentry
development_indicators = [
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (bug_risk): Brittle substring matching for development errors

Matching exception messages by substring may lead to false positives. Use exception types or structured error codes for more reliable filtering.

SUPPORTED_PLATFORMS = sorted([*set(PLATFORM_MAPPINGS.values()), "common"])


class TldrClient:
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (complexity): Consider splitting the monolithic TldrClient into four focused modules—cache, HTTP, parsing, and formatting—to improve maintainability and testability.

Here’s one possible way to tame the 1 700-line monolith by splitting responsibilities into four very focused modules. This keeps all existing behaviour but makes each piece small and test-friendly.


1) Extract the cache logic (cache.py)

# cache.py
import time
from pathlib import Path

class CacheManager:
    def __init__(self, base_dir: Path, max_age_hours: int):
        self.base = base_dir
        self.max_age = max_age_hours * 3600

    def _path(self, command: str, platform: str, lang: str) -> Path:
        pages = f"pages{f'.{lang}' if lang!='en' else ''}/{platform}"
        return self.base / pages / f"{command}.md"

    def is_recent(self, command, platform, lang) -> bool:
        p = self._path(command, platform, lang)
        if not p.exists(): return False
        return (time.time() - p.stat().st_mtime) <= self.max_age

    def read(self, command, platform, lang) -> str | None:
        try:
            return self._path(command, platform, lang).read_text(encoding="utf-8")
        except OSError:
            return None

    def write(self, content, command, platform, lang) -> None:
        p = self._path(command, platform, lang)
        p.parent.mkdir(parents=True, exist_ok=True)
        p.write_text(content, encoding="utf-8")

2) Extract HTTP fetching (http_client.py)

# http_client.py
from urllib.request import Request, urlopen
from urllib.error import HTTPError, URLError

class HttpClient:
    def __init__(self, timeout: int):
        self.timeout = timeout

    def fetch(self, url: str) -> str | None:
        req = Request(url, headers={"User-Agent": "tldr-python-client"})
        try:
            with urlopen(req, timeout=self.timeout) as resp:
                return resp.read().decode("utf-8")
        except (HTTPError, URLError):
            return None

3) Extract placeholder parsing (parser.py)

# parser.py
import re

class PlaceholderParser:
    def __init__(self, highlight: bool = True):
        self.highlight = highlight
        self._esc_open  = "__ESC_OPEN__"
        self._esc_close = "__ESC_CLOSE__"

    def parse(self, line: str, show_short: bool, show_long: bool, show_both: bool) -> str:
        line = line.replace(r"\{\{", self._esc_open).replace(r"\}\}", self._esc_close)
        def repl(m):
            c = m.group(1)
            if c.startswith("[") and "|" in c:
                a, b = c[1:-1].split("|",1)
                chosen = a if show_short else (b if show_long else f"{a}|{b}")
            else:
                chosen = c
            if self.highlight and not chosen.lstrip().startswith("-"):
                return f"__{chosen}__"
            return chosen
        out = re.sub(r"\{\{(.*?)\}\}", repl, line)
        return out.replace(self._esc_open, "{{")).replace(self._esc_close, "}}")

4) Extract Discord formatting (formatter.py)

# formatter.py
from parser import PlaceholderParser

class DiscordFormatter:
    def __init__(self):
        self.parser = PlaceholderParser()

    def format_page(self, md: str, show_short=False, show_long=True, show_both=False) -> str:
        lines = md.splitlines()
        # … use parser.parse(...) and private helpers to reassemble …
        # this snippet shows intent: everything Discord-specific stays here
        formatted = []
        for L in lines:
            formatted.append(self.parser.parse(L, show_short, show_long, show_both))
        return "\n".join(formatted)

5) Now TldrClient pulls it all together

# client.py
from cache import CacheManager
from http_client import HttpClient
from formatter import DiscordFormatter

class TldrClient:
    def __init__(self, cache_dir, max_age, timeout):
        self.cache = CacheManager(cache_dir, max_age)
        self.http  = HttpClient(timeout)
        self.fmt   = DiscordFormatter()

    def fetch_tldr_page(self, command, langs, platform=None):
        # 1. build urls for each (lang, platform)
        # 2. if self.cache.is_recent(...) return self.cache.read(...)
        # 3. else data = self.http.fetch(url); self.cache.write(data, ...); return data
        pass

    def format_for_discord(self, md, **opts):
        return self.fmt.format_page(md, **opts)

Benefits

  • Each file is under ~100 LOC.
  • You can unit-test CacheManager, HttpClient, PlaceholderParser and DiscordFormatter in isolation.
  • TldrClient becomes a thin coordinator.

This refactoring preserves every line of logic you added, but dramatically reduces nested branches and file length.

try:
# Add a small delay to ensure file write is complete
time.sleep(0.1)
time.sleep(0.1) # Ensure file write is complete
Copy link
Contributor

Choose a reason for hiding this comment

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

security (arbitrary-sleep): time.sleep() call; did you mean to leave this in?

Source: opengrep

@kzndotsh kzndotsh self-assigned this Jun 2, 2025
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.

[FEATURE] - Comply with the tldr specifications
1 participant