Implement CLI Configuration Loading and Option Handling Improvements #349
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @devtedlee, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've implemented a new configuration system for the Fedify CLI, allowing users to define persistent settings in .fedifyrc or fedify.config.json files. This change also centralizes and refactors how command-line options are handled, establishing a clear precedence (CLI flags > config file > hardcoded defaults) to improve the CLI's clarity, maintainability, and overall user experience.
Highlights
- Configuration System: Introduced a new configuration system that allows the Fedify CLI to load settings from .fedifyrc or fedify.config.json files, enabling persistent user-defined options.
- Option Precedence: Established a clear hierarchy for option resolution: command-line flags take precedence over configuration file settings, which in turn override hardcoded defaults.
- Centralized Option Handling: Refactored the CLI's core to include a globalAction in mod.ts that manages shared options and initializes resources like the cache directory, reducing duplication and improving consistency.
- --no-config Flag: Added a --no-config flag to allow users to bypass the loading of configuration files, providing greater flexibility for scripting and testing.
- Relocated Options: Moved several global options, such as --user-agent, to their respective subcommands (lookup, nodeinfo, webfinger) to make the CLI interface more intuitive and self-documenting.
- Logging Improvements: Centralized log level management within the globalAction based on --debug and --verbose flags, ensuring consistent logging behavior across all commands.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
github-actions
bot
added
component/cli
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable configuration file system for the Fedify CLI, which significantly improves usability. The implementation is well-structured, particularly the centralized option handling. My review focuses on enhancing this new system by ensuring all defined configuration options are utilized, suggesting a more robust and maintainable approach for the --no-config flag, and addressing a couple of bugs and redundancies. These changes will help make the configuration feature more complete and reliable.
| .globalAction(async (options) => { | ||
| const config = await loadConfig(); | ||
| setSharedOptions(config); |
There was a problem hiding this comment.
The --no-config flag is implemented on a per-command basis, which leads to some code duplication and a more complex implementation for sharing options. For better clarity, maintainability, and user experience, consider making --no-config a true global option.
This would involve:
- Adding
.globalOption("--no-config [flag:boolean]", "Disable loading config file.")inmod.ts. - Changing
const config = await loadConfig();toconst config = options.noConfig ? {} : await loadConfig();in theglobalAction. - Removing the
--no-configoption from all subcommands (lookup,nodeinfo,webfinger). - Simplifying
getSharedOptioninpackages/cli/src/utils.tsby removing thenoConfigparameter, as the logic will be handled centrally.
packages/cli/src/lookup.ts
Outdated
| const collection = await lookupObject(url, { | ||
| documentLoader: authLoader ?? documentLoader, | ||
| contextLoader, | ||
| userAgent: options.userAgent, | ||
| userAgent: options.userAgent ?? | ||
| getSharedOption("userAgent", options.noConfig), | ||
| }); |
There was a problem hiding this comment.
The timeout and followRedirects settings from the configuration file are not being applied when calling lookupObject. You should pass these options to ensure the CLI respects the user's configuration. For full consistency, you might also consider adding --timeout and --follow-redirects flags to this command.
const collection = await lookupObject(url, {
documentLoader: authLoader ?? documentLoader,
contextLoader,
userAgent: options.userAgent ??
getSharedOption("userAgent", options.noConfig),
timeout: getSharedOption("timeout", options.noConfig),
followRedirects: getSharedOption("followRedirects", options.noConfig),
});
packages/cli/src/nodeinfo.ts
Outdated
| const nodeInfo = await getNodeInfo(url, { | ||
| parse: "none", | ||
| userAgent: options.userAgent, | ||
| userAgent: options.userAgent ?? | ||
| getSharedOption("userAgent", options.noConfig), | ||
| }); |
There was a problem hiding this comment.
The timeout setting from the configuration file is not being used when calling getNodeInfo. Please pass it through to respect the user's configuration.
const nodeInfo = await getNodeInfo(url, {
parse: "none",
userAgent: options.userAgent ??
getSharedOption("userAgent", options.noConfig),
timeout: getSharedOption("timeout", options.noConfig),
});
packages/cli/src/nodeinfo.ts
Outdated
| const nodeInfo = await getNodeInfo(url, { | ||
| parse: options.bestEffort ? "best-effort" : "strict", | ||
| userAgent: options.userAgent, | ||
| userAgent: options.userAgent ?? | ||
| getSharedOption("userAgent", options.noConfig), | ||
| }); |
There was a problem hiding this comment.
The timeout setting from the configuration file is not being used when calling getNodeInfo. Please pass it through to respect the user's configuration.
const nodeInfo = await getNodeInfo(url, {
parse: options.bestEffort ? "best-effort" : "strict",
userAgent: options.userAgent ??
getSharedOption("userAgent", options.noConfig),
timeout: getSharedOption("timeout", options.noConfig),
});
packages/cli/src/nodeinfo.ts
Outdated
| const response = await fetch(faviconUrl, { | ||
| headers: { | ||
| "User-Agent": options.userAgent == null | ||
| ? getUserAgent() | ||
| : options.userAgent, | ||
| "User-Agent": options.userAgent ?? | ||
| getSharedOption("userAgent", options.noConfig) ?? | ||
| getUserAgent(), | ||
| }, | ||
| }); |
There was a problem hiding this comment.
The followRedirects setting from the configuration file is not being applied to this fetch call. You can use this setting to control the redirect option for fetch.
const response = await fetch(faviconUrl, {
headers: {
"User-Agent": options.userAgent ??
getSharedOption("userAgent", options.noConfig) ??
getUserAgent(),
},
redirect: getSharedOption("followRedirects", options.noConfig) === false
? "manual"
: "follow",
});
packages/cli/src/webfinger.ts
Outdated
| const webFinger = await lookupWebFinger(url, { | ||
| ...options, | ||
| userAgent: options.userAgent ?? | ||
| getSharedOption("userAgent", options.noConfig), | ||
| }) ?? // Look up WebFinger | ||
| new NotFoundError(resource).throw(); // throw NotFoundError if not found |
There was a problem hiding this comment.
The timeout setting from the configuration file is not being applied when calling lookupWebFinger. Please pass it through to respect the user's configuration.
| const webFinger = await lookupWebFinger(url, { | |
| ...options, | |
| userAgent: options.userAgent ?? | |
| getSharedOption("userAgent", options.noConfig), | |
| }) ?? // Look up WebFinger | |
| new NotFoundError(resource).throw(); // throw NotFoundError if not found | |
| const webFinger = await lookupWebFinger(url, { | |
| ...options, | |
| userAgent: options.userAgent ?? | |
| getSharedOption("userAgent", options.noConfig), | |
| timeout: getSharedOption("timeout", options.noConfig), | |
| }) ?? // Look up WebFinger | |
| new NotFoundError(resource).throw(); // throw NotFoundError if not found |
|
The docs for this pull request have been published: |
devtedlee
force-pushed
the
feat/cli-config
branch
from
d786a2a to
3fe2e57
Compare
There was a problem hiding this comment.
This behavior should be documented in the docs/cli.md file. It should include:
- Paths where the
fedifycommand looks for the configuration files (on Linux/macOS/Windows) - Available configuration fields
- Who wins when there are different settings are provided through CLI options and configuration files
- etc
- add Config interface for options - add loadConfig function for load files like .fedifyrc and .fedify.config.json - add tests for config.ts - chore: remove duplicated std/assert package, and add std/path package # Conflicts: # packages/cli/src/config.test.ts # packages/cli/src/config.ts
- apply logger at config.ts - add global command options: userAgent, timeout, redirects, verbose, format, noConfig - command option is higher priority than config file options # Conflicts: # cli/mod.ts
The new approach introduces a single `globalAction` that: - Checks for the `--no-config` flag to conditionally load external configuration. - Merges options from different sources with the correct precedence: 1. Command-line arguments 2. Configuration file values 3. Hardcoded defaults - Initializes shared resources like the cache directory (`setCacheDir`) after all options have been resolved. - apply this sharedOptions to lookup and webfinger
This commit overhauls the previously confusing global command-line options, relocating them to more appropriate subcommands. It also introduces a feature to bypass config file loading, enhancing the CLI's flexibility and testability. Key changes include: - Option Relocation: Removed global options like --user-agent and moved them to individual subcommands (lookup, nodeinfo, webfinger) for better clarity. - '--no-config' Flag: Introduced a --no-config option to skip reading the configuration file. The getSharedOption utility has been updated to respect this flag. - Improved Logging Logic: Centralized the log level configuration based on --debug and --verbose flags into the globalAction for more consistent handling. - Code Simplification: Reducing global options has simplified the globalAction logic in mod.ts. Each subcommand is now responsible only for its own options, clarifying the structure.
- add 2 tests for config.test.ts
- mod.ts: remove unnecessary cache dir, add options.verbose override logic - lookup.ts: add missing code for getSharedOption for userAgent
- remove vocab.ts file
- fix by PR - 'XDG_CONFIG_HOME' applied
- use 'options.config' instead of 'options.noConfig'
- fix by PR - modify logtape log level to 'warning' - add global option no-config
- at window: default AppData, if not use UserProfile - at linux: defualt XDG_CONFIG_HOME, if not use Home
- fix failed tests - add tests for os specific logic
- below contents added. - global option for commands - paths where the fedify command looks for the configuration files (on Linux/macOS/Windows) - available configuration fields - who wins when there are different settings are provided through CLI options and configuration files
devtedlee
force-pushed
the
feat/cli-config
branch
from
4f523c6 to
66f8107
Compare
|
The latest push to this pull request has been published to JSR and npm as a pre-release:
|
| /** http request timeout */ | ||
| timeout?: number; | ||
| /** auto follow redirects mode */ | ||
| followRedirects?: boolean; |
There was a problem hiding this comment.
Hmm… those two fields are apparently used nowhere?
| /** HTTP related configuration */ | ||
| http?: HttpConfig; | ||
| /** Output format configuration */ | ||
| format?: FormatConfig; |
There was a problem hiding this comment.
yes, I also noticed that at PR description 'focus on review' section. I don't know where to use this fields precisely.
I think If we try to use this fields, we also modify other fedify related codes. like @fedify/fedify getDocumentLoader function
There was a problem hiding this comment.
See also the writeObjectToStream() function which is used in the packages/cli/src/lookup.ts file.
Summary
This pull request introduces a configuration file system for the Fedify CLI, allowing users to set persistent options in
.fedifyrcorfedify.config.json. It also centralizes and refactors command-line option handling to improve structural clarity, maintainability, and user experience by establishing a clear priority for settings.Related Issue
Changes
packages/cli/src/config.tswith an interface for configuration and aloadConfigfunction to read.fedifyrcandfedify.config.jsonfiles.globalActioninmod.tsto manage shared options and resource initialization (e.g., cache directory).--no-configflag to allow bypassing the configuration file loading.--user-agent) to their relevant subcommands (lookup,nodeinfo,webfinger).--debugand--verboseflags within theglobalAction.Benefits
globalActionreduces code duplication and simplifies future development.--no-configflag enables running the CLI in an isolated state, which is crucial for scripting, automation, and reliable testing.Checklist
Additional Notes
This work was done incrementally, starting with adding the core config loading feature and then refactoring the option parsing logic for better structure. The changes affect the core CLI module and several subcommands, establishing a new, more robust foundation for how the CLI is configured.
Focus on Review