-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
Conversation
… 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.
Reviewer's GuideThis 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 ExecutionsequenceDiagram
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
Sequence Diagram for Hot Reload File Change EventsequenceDiagram
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
Class Diagram for TLDR System ComponentsclassDiagram
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
Class Diagram for Enhanced Hot Reload SystemclassDiagram
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
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Deploying tux with
|
Latest commit: |
1782eb1
|
Status: | ✅ Deploy successful! |
Preview URL: | https://c6ab3fe8.tux-afh.pages.dev |
Branch Preview URL: | https://tldr-rewrite.tux-afh.pages.dev |
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.
There was a problem hiding this 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 torun_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
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: |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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
andhash_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: |
There was a problem hiding this comment.
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 theimported_name
argument (e.g., the name being imported infrom . 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.
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}") |
There was a problem hiding this comment.
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.
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}") |
# 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] |
There was a problem hiding this comment.
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.
# 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 = [ |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
… 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:
Enhancements:
CI: