Skip to content

fix(editor): highlight scoped dotenv filenames on desktop and mobile - #22416

Merged
nwparker merged 6 commits into
mainfrom
nwparker/external-13028-dotenv-highlighting
Sep 26, 2026
Merged

nwparker merged 6 commits into
mainfrom
nwparker/external-13028-dotenv-highlighting

Conversation

@nwparker

@nwparker nwparker commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​90 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​90
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​12 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​10

ELI5

Environment files such as .env.staging and .env.functions.local now receive syntax colors on desktop and mobile.

What Changed

Add a basename fallback for .env and .env.* to the existing INI grammar in both language detectors. Exact names, known final extensions (for example .env.sh) and mobile language preferences retain priority.

Why

The existing four-name allowlist misses deployment-specific names. A fallback reuses both existing detectors and grammars without broadening *.env or adding another shared detection layer. Adapted from @WillyDallas in #13029 and @innocarpe in #13239; both receive commit attribution.

Linked Issue

Fixes #13028.

Visual Proof

Real hidden macOS file editor before/after, captured on baseline da1c322b000 with this same fallback during the prior validation pass. Cropped to exclude local paths. Desktop file and diff behavior and headless mobile component rendering were also checked; the mobile capture used synthetic component props, not a device or runtime connection.

13028-before.png
13028-after.png

Testing

  • I manually tested these changes locally
  • Automated tests added/updated

Current-base detector tests: 42 desktop and 37 mobile pass. Renderer types and changed-code quality pass. Mobile production types pass. Earlier hidden desktop and headless mobile file/diff checks passed; broad mobile test types retain 484 baseline-identical diagnostics.

The earlier RPC recording pin failure was fixed upstream in #22407. This branch includes that upstream repair; all 79 focused desktop/mobile tests and changed-code quality pass after that merge, and full-history CI passed the RPC pin gate. No recording corpus pin or golden was edited by this PR.

The mobile bundle CI exposed an inherited stale module-count assertion: branch and exact base both measured 4,216 modules (1,030 local) with identical module lists. Upstream #22430 has now landed the same census repair. This branch includes main at 293c2508fcd, retains the upstream census, and changes only the four intended dotenv files. The dotenv files are unchanged by this merge. At 389e98a2f7b7, both PR Checks and Mobile Checks passed, including their separate verification aggregates; 79 detector tests, five closure tests and changed-code quality also passed locally after the merge.

Review

Reviewed filename/extension precedence, case variants, Windows and POSIX path boundaries, language-preference priority and desktop/mobile agreement. The test cases are placed separately from the independent Apex and ABAP changes so they can merge in any order.

Agent skill upstream boundary

  • Not applicable; no agent skill source changes.

Notes

Local validation did not include native Windows/Linux, live SSH, a native mobile device, the full suite or release packaging. CI provides the broader tests and packaging coverage recorded above. File transport, host selection, Git providers and remote wire behavior are unchanged.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • Before/after screenshots attached
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered
  • CI static analysis, typecheck, test and packaging pass (focused local checks above)

nwparker and others added 3 commits September 22, 2026 22:37
Co-authored-by: willydallas <willy.dallas@pm.me>
Co-authored-by: Wooseong Kim <innocarpe@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 197f03fb-fc07-4697-8e8d-0c7de14b8ebf

📥 Commits

Reviewing files that changed from the base of the PR and between 293c250 and 389e98a.

📒 Files selected for processing (4)
  • mobile/src/session/mobile-file-language.ts
  • mobile/src/session/mobile-file-syntax.test.ts
  • src/renderer/src/lib/language-detect.test.ts
  • src/renderer/src/lib/language-detect.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

Desktop and mobile language detection now map .env and .env.* filenames to ini when no recognized extension mapping applies. Existing extension mappings retain precedence, and other unmatched filenames remain plaintext. Tests cover filename matching, explicit mobile language preferences, and highlighting dotenv content and diff lines with the mobile ini grammar.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 389e9

The scoped dotenv highlighting change appears ready to merge after normal checks; no actionable merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR satisfies the coding requirements in issue #13028. Desktop and mobile detection retain exact-name and extension precedence, then map .env and .env.* basenames to ini. Known names such as …
Out of Scope Changes check ✅ Passed The reviewed diff contains only the desktop and mobile detector changes and related regression tests. The changes directly support issue #13028. No unrelated production behavior or unrelated test chan…
Title check ✅ Passed The title clearly summarizes the main change: dotenv filename highlighting in desktop and mobile editors.
Description check ✅ Passed The description covers the change, reason, linked issue, visual proof, testing, review, and checklist. It also states testing limits and platform coverage.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Dotenv fallback (desktop) — detectLanguage now falls back to ini when the lowercased basename is .env or starts with .env., after exact-filename and extension lookups.
  • Dotenv fallback (mobile) — detectMobileFileLanguage gains the identical fallback, preserving preferredLanguage precedence.
  • Desktop tests — a 26-case it.each table covering scoped names, case variants, Windows/UNC paths, extension precedence, and non-dotenv near-misses.
  • Mobile tests — the same detection table plus an end-to-end proof that the ini grammar highlights file and diff content.

The precedence contract holds: .env.sh → shell, .env.json → json, .env.local.ts → typescript, .env/CMakeLists.txt → cmake, and non-dotenv names (.envrc, .environment, dev.env, other.env.local, ..env.local) stay plaintext. .ini was already a live language id on desktop, and ini is in lowlight's common bundle, so no grammar registration is needed. The tests pin exact values rather than loose assertions, so they would have failed before the fallback existed.

I verified desktop tests pass locally (42) and independently confirmed lowlight's ini grammar emits the asserted comment/variable/string kinds. Mobile tests could not be run in this sandbox because mobile/node_modules is absent (expo/tsconfig.base.json unresolved), but the detection table was traced by hand against the extname semantics and matches.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@nwparker

Copy link
Copy Markdown
Contributor Author

Main has now landed the same 4216-module census repair in #22430 (293c2508fcd3a8efc9e40bfe0ed0061d9606291b). GitHub reports a merge conflict with this PR's equivalent local census update. The latest GraphQL base SHA still showed the older main, so the conflict was checked against the actual remote main ref and fetched locally.

Resolve by retaining the landed census file and preserving the four-file dotenv highlighting change; the measured module graph is unchanged. The independent census repair no longer needs to appear in this PR's diff.

@nwparker
nwparker merged commit 77bc0d7 into main Sep 26, 2026
32 checks passed
@nwparker
nwparker deleted the nwparker/external-13028-dotenv-highlighting branch September 26, 2026 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Non-standard .env files (.env.functions.local, .env.staging, …) render as plaintext

1 participant