Enhance Variable Management in Hurl.nvim #228
Conversation
![sweep-ai[bot]](https://avatars.githubusercontent.com/in/307814?s=60&v=4){kind=small}
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
❌ Error while fixing CITracebackSweep was unable to fetch CI/CD logs for jellydn/hurl.nvim due to GitHub errors. Sweep has encountered a runtime error unrelated to your request. Please let us know via this link or at support@sweep.dev directly. 📖 For more information on how to use Sweep, please read our documentation. Tracking ID: 94fc04369f-ci-0
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughThe changes enhance the HurlManageVariable command in the Hurl.nvim plugin. The command now retrieves persisted variables from storage and loads environment file variables, merging them with precedence for persisted values. The user interface prompts have been updated to clarify actions such as creating, editing, and deleting variables—with deletion of environment variables prevented. Additionally, new utility functions for parsing environment files and managing persistent JSON storage are introduced. The test suite now includes cases to verify env file loading, variable persistence, and the merged display. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant H as HurlManageVariable Command
participant UT as Utils Module
participant S as Storage/Env File
U->>H: Trigger :HurlManageVariable
H->>UT: load_persisted_vars()
H->>UT: parse_env_file()
UT-->>H: Return persisted and env variables
H->>H: Merge variables (persisted wins)
H->>U: Display UI with merged variables
U->>H: Edit/Delete/Add variable
H->>UT: save_persisted_vars(updated variables)
U->>H: Confirm changes
Assessment against linked issues
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
for more information, see https://pre-commit.ci
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (3)
- lua/hurl/main.lua: Language not supported
- lua/hurl/utils.lua: Language not supported
- test/plugin_spec.lua: Language not supported
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
lua/hurl/main.lua (1)
308-329: Consider sorting newly created variables.Appending a new variable at the bottom may lead to confusion for larger sets of variables. For better readability and consistency, you might want to insert the variable in alphabetical order or re-sort the entire list whenever a variable is created.
test/plugin_spec.lua (1)
31-41: Add broader test coverage for environment file parsing.Tests currently check a simple key-value pair. To fortify reliability, consider verifying corner cases (e.g., quoted values, no value after “=”, “export” prefix, or comments with leading spaces) to ensure robust parsing in different real-world scenarios.
lua/hurl/utils.lua (2)
278-301: Consider supporting more advanced .env formats.Currently, you only parse “KEY=VALUE” lines. Real-world .env files sometimes contain quotes, multiline values, or “export KEY=VALUE” forms. If you want broader flexibility, consider extending this parser or clarifying limitations in docs.
314-350: Storing variables in plain JSON may pose security risks.Loading and saving JSON is fine for typical use cases, but be aware that secrets in plain text can be compromised. If needed, consider encrypting sensitive data under a key or restricting file permissions to mitigate potential leaks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lua/hurl/main.lua(1 hunks)lua/hurl/utils.lua(1 hunks)test/plugin_spec.lua(1 hunks)
🔇 Additional comments (6)
lua/hurl/main.lua (3)
248-265: Confirm variable precedence and consider secure storage.You’re merging environment variables into persisted variables with “force,” giving persisted vars the highest precedence. Please verify that this is the desired order if environment variables are supposed to override local changes. Also, be mindful that storing secrets in plain text could be risky if these variables contain sensitive data. You might consider a more secure storage strategy (e.g., file-level permissions, encryption) to mitigate potential leaks.
292-306: Editing flow looks good—ensure test coverage.The edit logic is straightforward; it directly updates persisted variables and the buffer line. This flow is essential to test under various scenarios (e.g., incomplete or canceled input). If not covered yet, consider adding integration tests for the popup-based edit flow to ensure it’s robust.
331-346: Environment variable deletion prevention is commendable.This logic correctly prevents removing environment-populated variables. Users receive a warning and are blocked from such deletions. This approach helps avoid accidental misconstruction of environment states.
test/plugin_spec.lua (2)
43-49: Persist and reload logic looks correct.The test effectively ensures variables remain consistent across sessions, covering writing and reading from storage. Great job adding this coverage.
51-71: Verify handling of conflicts between env and persisted variables.Your test checks a simple merge scenario. Consider adding a test case where the same variable is defined in both env and persisted sets, then confirm your precedence logic is enforced properly.
lua/hurl/utils.lua (1)
303-312: Creating and verifying storage directory works well.The code reliably checks for the directory, creates it if absent, then returns a usable path. This is straightforward and effective.
Purpose
Improve variable management in Hurl.nvim by introducing persistent variable storage, supporting environment file parsing, and providing a more robust variable management interface.
This pull request was created to solve the following GitHub issue:
The
:HurlManageVariablecommand doesn't persist values (and doesn't prepopulate fromenv_file)Details
No response
Enhance
:HurlManageVariablewith persistence and env file integrationDescription:
The
:HurlManageVariablecommand currently lacks persistence between sessions and doesn't load initial values from the configured env files. This needs to be fixed to improve variable management functionality.Tasks:
In
lua/hurl/main.lua:HurlManageVariablecommand to load variables from env files on initializationIn
lua/hurl/lib/hurl_runner.lua:In
test/plugin_spec.lua:Test:
test/plugin_spec.lua:Add persistence and env file integration to HurlManageVariable
Description:
Enhance the
:HurlManageVariablecommand to persist variables between sessions and load initial values from configured env files.Tasks:
In
lua/hurl/main.lua:_HURL_GLOBAL_CONFIG.global_varsHurlManageVariablecommand to:vim.fn.stdpath('data')/hurl-nvim/variables.jsonIn
lua/hurl/utils.lua:In
test/plugin_spec.lua:Implementation Notes:
Description
This pull request introduces several key enhancements to variable management:
HurlManageVariablecommand with new features:The changes provide users with a more flexible and user-friendly way to manage variables across different Hurl.nvim sessions and environments.
Summary
New Features
Modified
fileslua/hurl/main.lua: Updated variable management commandlua/hurl/utils.lua: Added utility functions for variable handlingtest/plugin_spec.lua: Added tests for new variable management functionalityNew Capabilities
.envfilesFixes
Fixes #219. Continue the conversation here: https://app.sweep.dev/c/9916350e-5ad7-45c4-a067-e0933c3c11e0.
To have Sweep make further changes, please add a comment to this PR starting with "Sweep:".
📖 For more information on how to use Sweep, please read our documentation.
Summary by CodeRabbit
New Features
Tests