feat: add env support to Java profiles#483
Conversation
s1n7ax
left a comment
s1n7ax
left a comment
There was a problem hiding this comment.
Nice feature — env-from-profile is a very natural fit. Solid tests too. A few things to look at:
Env replaces, not merges, the process environment — lua/java-runner/run.lua
vim.fn.jobstart({ env = env }) replaces the child env with exactly what's in env (PATH, HOME, etc. are gone unless inherited explicitly by some Neovim versions — behavior here is fragile). At minimum, please merge with vim.fn.environ() before passing, otherwise the launched process will lose PATH and many Java apps will misbehave. Worth a test covering this.
Marker mutation may snowball — lua/java-dap/init.lua
Setting config._nvim_java_managed = true on the config object works for newly created entries. But nvim_dap.configurations.java items can be the same table objects across calls; once you've set the marker on a config, even if a user later edits it, it stays marked. Consider tracking managed configs in a side table keyed by reference, or by (name, request) tuples, rather than mutating user data.
is_absolute_path — lua/java/utils/env.lua
Doesn't cover \\ (just Windows backslash) absolute paths, only UNC \\\\… and forward-slash. On Windows C:\foo is matched by ^%a:[/\\], good — but a path like \foo\bar (drive-relative) would resolve against base_dir, which may or may not be intended. Minor; please at least document.
Env key regex too permissive — parse_line
[%w_.%-]+ allows dots and dashes in keys. POSIX env names are [A-Za-z_][A-Za-z0-9_]*. Java will accept most things but downstream tools won't. Tightening avoids weird-looking profiles slipping through.
Missing dotenv semantics — normalize_value
No escape handling (\n, \"), and trailing comments aren't stripped (KEY=value # note). If you want this to match what users paste from .env files used with IntelliJ, both are common. Worth at least documenting the supported subset.
CLAUDE.md addition
Profile blurb in CLAUDE.md is a nice touch — but per project conventions ("Be extremely concise"), consider trimming to one short line.
s1n7ax
left a comment
s1n7ax
left a comment
There was a problem hiding this comment.
Nice feature, solid test coverage. A few correctness/robustness asks inline.
| self:send_term(merged_cmd) | ||
|
|
||
| self.job_chan_id = vim.fn.jobstart(merged_cmd, { | ||
| env = env, |
There was a problem hiding this comment.
vim.fn.jobstart({ env = env }) replaces the child environment with exactly this table — PATH, HOME, JAVA_HOME, etc. are lost. Many Java apps will misbehave (no PATH means no java resolution on some platforms; tools that shell out break entirely). Please merge with vim.fn.environ() before passing, e.g. env = vim.tbl_extend('force', vim.fn.environ(), env or {}). Worth a test covering this.
| end | ||
|
|
||
| if is_valid then | ||
| config._nvim_java_managed = true |
There was a problem hiding this comment.
Mutating user data: config._nvim_java_managed = true is set on the configuration object after the user-config preservation loop runs. That's fine for newly created configs, but nvim_dap.configurations.java can contain reused table references (some users register the same table object). Once you mark a config it stays marked across runs even if a user later promotes it to their own. Safer to track managed entries in a side table keyed by reference, or by (name, request) tuple, rather than mutating the data passed in.
| ---@param line string | ||
| ---@return string|nil | ||
| ---@return string|nil | ||
| function M.parse_line(line) |
There was a problem hiding this comment.
Key regex [%w_.%-]+ is too permissive — POSIX env names are [A-Za-z_][A-Za-z0-9_]*. Allowing dots/dashes lets invalid names through that Java will accept but most shells/tools downstream will not. Tightening avoids weird-looking profiles slipping through.
|
|
||
| ---@param value string | ||
| ---@return string | ||
| local function normalize_value(value) |
There was a problem hiding this comment.
Missing common dotenv semantics: no escape handling (\n, \"), and trailing comments aren't stripped (KEY=value # note keeps the comment in the value). If the goal is to match what users paste from .env files used with IntelliJ (per PR description), both are common. At minimum please document the supported subset in the README/help.
| vm_args = active_profile.vm_args or '' | ||
|
|
||
| local err | ||
| env, err = env_utils.load_profile_env( |
There was a problem hiding this comment.
Same env merge concern as run.lua — please document or ensure the merge happens at one consistent layer.
2 participants
Summary
JavaProfileWhy
Many Java applications, especially multi-tenant ones, rely on environment-driven
configuration layered into merged
application.propertiessetup. In practice,that means launch-time env variables are often the most natural source of truth.
I also often share env files with colleagues who use IntelliJ IDEA, where this
workflow is already common. Without this feature, those values had to be
manually converted into VM args or program args for Neovim, which is tedious,
error-prone, and hard to keep aligned across teams.
Profiles are already the place where launch-specific configuration lives, so
extending them to accept env values and env files keeps that workflow intact.
This makes it possible to paste env entries directly or reference an existing
env file as-is instead of translating it into another format.
Behavior