-
Notifications
You must be signed in to change notification settings - Fork 311
chore(qol): Copilot no longer uses instruction settings; it automatic… #6282
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
base: master
Are you sure you want to change the base?
Conversation
…ally detects instructions files and PRs.
… container's environment, so you don't need to use the syntax. Removed braces
…el commands and recurse through --help.\Both products work together: Running node [audit-cli-documentation.js](http://_vscodecontentref_/1) both successfully audits both Core and Enterprise Core templates use Core-specific frontmatter Enterprise templates use Enterprise-specific frontmatter Fixes audit-cli-documentation.js so that it parses commands dynamically from the CLI output. Some commands () only return top-level help output, which the script had some difficulty with.That seems mostly resolved, but might rear again.
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.
Pull Request Overview
This PR removes manual Copilot instruction settings from VS Code configuration and refactors the CLI documentation auditor to use dynamic command discovery instead of hardcoded command lists. The changes improve maintainability by automatically detecting available CLI commands and their subcommands from the actual CLI help output.
Key changes:
- Removed hardcoded command lists and replaced with dynamic command discovery
- Enhanced container management with automatic startup and status checking
- Improved CLI help parsing with better ANSI color code handling
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
helper-scripts/influxdb3-monolith/audit-cli-documentation.js | Refactored from static command lists to dynamic command discovery with improved container management and help parsing |
compose.yaml | Removed unused INFLUXDB3_ENTERPRISE_LICENSE_EMAIL environment variable |
.vscode/settings.json | Removed manual Copilot instruction settings to rely on automatic detection |
@@ -343,7 +544,7 @@ class CLIDocAuditor { | |||
const lines = content.split('\n'); | |||
let inCommand = false; | |||
let helpText = []; | |||
const commandHeader = `===== influxdb3 ${command} --help =====`; | |||
const commandHeader = `===== influxdb3 ${command} --help`; |
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.
The command header pattern is missing the closing '=====' delimiter. This will break the command extraction logic that expects a complete header pattern like '===== influxdb3 command --help ====='.
const commandHeader = `===== influxdb3 ${command} --help`; | |
const commandHeader = `===== influxdb3 ${command} --help =====`; |
Copilot uses AI. Check for mistakes.
// Look for any Commands section | ||
if (trimmed.includes('Commands:') || trimmed === 'Resource Management:' || | ||
trimmed === 'System Management:') { | ||
inCommandsSection = true; | ||
continue; | ||
} | ||
|
||
// Stop at next section (but don't stop on management sections) | ||
if (inCommandsSection && /^[A-Z][a-z]+:$/.test(trimmed) && | ||
!trimmed.includes('Commands:') && | ||
trimmed !== 'Resource Management:' && | ||
trimmed !== 'System Management:') { | ||
break; | ||
} | ||
|
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.
The hardcoded section names ('Resource Management:', 'System Management:') make the parser brittle. Consider using a more flexible pattern or making these configurable to handle variations in CLI help format.
// Look for any Commands section | |
if (trimmed.includes('Commands:') || trimmed === 'Resource Management:' || | |
trimmed === 'System Management:') { | |
inCommandsSection = true; | |
continue; | |
} | |
// Stop at next section (but don't stop on management sections) | |
if (inCommandsSection && /^[A-Z][a-z]+:$/.test(trimmed) && | |
!trimmed.includes('Commands:') && | |
trimmed !== 'Resource Management:' && | |
trimmed !== 'System Management:') { | |
break; | |
} | |
// Look for any Commands section or section header ending with ':' | |
if (/^[A-Za-z0-9 _-]+:$/i.test(trimmed)) { | |
// Enter commands section if it's a Commands section, or a known management section | |
if (trimmed.toLowerCase().includes('commands') || | |
trimmed.toLowerCase().includes('management')) { | |
inCommandsSection = true; | |
continue; | |
} else if (inCommandsSection) { | |
// If we're already in a commands section and hit a new section header, exit | |
break; | |
} | |
} |
Copilot uses AI. Check for mistakes.
this.commandOptionsMap[fullCommand] = options; | ||
|
||
// Recursively discover subcommands (but limit depth) | ||
if (subcommands.length > 0 && commandParts.length < 3) { |
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.
The magic number '3' for depth limiting should be defined as a named constant to improve code clarity and maintainability.
Copilot uses AI. Check for mistakes.
- Updated /content/influxdb3/core/reference/cli/influxdb3/serve.md with 84 Core-compatible options - Excluded 21 Enterprise-only options (like cluster-id, license-email, license-file, mode, compaction options, clustering options, etc.) - All options now link to the Core configuration documentation - Fixed the environment variables link to point to Core instead of Enterprise 2. Updated Enterprise serve.md Options Section - Updated /content/influxdb3/enterprise/reference/cli/influxdb3/serve.md with all 105 configuration options - Includes both Core and Enterprise-only options - Maintains proper required option markers for node-id and cluster-id - All options link to the Enterprise configuration documentation 3. Verified Content Consistency - Core serve.md: Contains examples without cluster-id parameter (appropriate for Core) - Enterprise serve.md: Contains examples with both node-id and cluster-id parameters, plus Enterprise-specific mode examples - Both maintain consistent structure and troubleshooting sections appropriate to their respective products - Environment variables sections correctly reference their respective configuration documentation 4. Allow Vale to accept "parquet" in lowercase when it appears in: - Command-line options (e.g., --parquet-mem-cache-size) - Hyphenated configuration names (e.g., parquet-mem-cache-prune-interval) - Code blocks or inline code (e.g., `parquet`) Key Changes Made: - Core: Now includes 84 options (was 69), excluding Enterprise-only features - Enterprise: Now includes all 105 options (was 78), comprehensive coverage - Alphabetical ordering: Both option tables are now properly alphabetized - New options added: Many previously missing options like buffer-mem-limit-mb, tcp-listener-file-path, telemetry-*, wal-replay-*, etc. - Proper segregation: Core users no longer see Enterprise-only configuration options - Vale allows Parquet or parquet in the appropriate context
…and grep docs for commands- replaces the CLI audit script- searches tagged repo branch source code for CLI commands- searches docs content the commands- allows including and excluding "categories" of docs paths to search
…ally detects instructions files and PRs.
Closes #
Describe your proposed changes here.
(if necessary)