Integrate the new nf-lang formatter (comment preservation, fmt directives, line wrapping)#163
Open
ewels wants to merge 7 commits into
Open
Integrate the new nf-lang formatter (comment preservation, fmt directives, line wrapping)#163ewels wants to merge 7 commits into
ewels wants to merge 7 commits into
Conversation
…xtflow#7346) Points the build at the nf-lang formatter overhaul from PR nextflow-io/nextflow#7346, published to Maven local from the PR head branch. This commit must be amended to the released nf-lang version once the PR is merged and an nf-lang release exists — do not ship it as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb
Expose the new nf-lang line-wrapping option as nextflow.formatting.maxLineLength (default FormattingOptions.DEFAULT_MAX_LINE_LENGTH = 120, 0 disables wrapping) and pass it explicitly into every FormattingOptions construction, so the language server never relies on the 5-arg constructor's implicit default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb
…omment loss Use the new 3-arg visitor constructors so comment reattachment always sees exactly the buffer being formatted instead of re-reading the SourceUnit. Adopt the same safety check as 'nextflow lint -format': compare CommentReattacher.commentTexts before and after formatting and refuse to return any edit if a comment would be removed or altered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb
Pin the new canonical style at the language-server level: K&R if/else, blank-line normalization, comment preservation (trailing, dangling and commented-out declarations), line wrapping at the configured maximum line length, and fmt: skip / fmt: off / fmt: on directives. Add integration tests for the language-server trigger scenarios: - formatting the same cached AST repeatedly without a document change produces identical output (comment metadata re-derivation must be idempotent) - maxLineLength=0 disables line wrapping - a file using fmt: off/on round-trips unchanged - converting a script to static types after formatting the same cached AST neither crashes nor duplicates comments (the conversion formats freshly-built AST nodes that carry no reattached comment metadata) The cached-AST tests write the file to disk first: the deferred workspace scan re-scans from disk and would otherwise evict an in-memory-only file from the AST cache between requests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb
…ents Pin the language-server-side behavior for the remaining formatter issues closed by nextflow-io/nextflow#7346: include sorting under sortDeclarations (#54), multi-line string re-indentation (#116), and leading comments in CRLF files no longer merging with the following code (#127). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb
The Nextflow docs moved from nextflow.io/docs/latest to docs.seqera.io/nextflow upstream, so the doc links baked into nf-lang 26.07.0-edge annotations changed accordingly. This change is durable: it applies to any nf-lang release cut from current master, independent of the formatter overhaul. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb
- Extract the comment-preservation check into a shared FormattingProvider.commentsPreserved helper so the script and config providers no longer duplicate the safety-critical guard, and return no edits when the formatted text is unchanged instead of a whole-document no-op edit. - Move the file-on-disk workaround for the deferred workspace scan into TestUtils (openOnDisk/deleteOnDisk) so the eviction rationale is documented once instead of in three test classes. - Rebuild ConfigFormattingTest on the shared TestUtils harness instead of a private service-bootstrap clone. - Collapse round-trip test cases onto checkRoundTrip helpers instead of pasting identical literals twice, and make the cached-AST tests use non-canonical input so a refused format cannot be mistaken for a no-op. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prepares the language server to consume the nf-lang formatter overhaul from nextflow-io/nextflow#7346: exposes the new
nextflow.formatting.maxLineLengthoption, passes the open document text to the formatting visitors, adds the same refuse-to-format-on-comment-loss guard asnextflow lint, and updates the test suite for the new canonical style.Do not merge as-is: the first commit temporarily points
build.gradleat a locally-builtnf-lang 26.07.0-edgeviamavenLocal(). It must be amended to a released nf-lang version once PR #7346 merges and a release is cut.Changes
maxLineLength option
maxLineLengthfield onLanguageServerConfiguration(defaultFormattingOptions.DEFAULT_MAX_LINE_LENGTH= 120,0disables wrapping), read from thenextflow.formatting.maxLineLengthsetting alongside the existingharshilAlignment/maheshFormwiring.FormattingOptionsconstruction (NextflowLanguageServer.formatting(),ScriptService.formattingOptions()) rather than relying on the 5-arg constructor's implicit default.Source text and safety guard
ScriptFormattingProviderandConfigFormattingProvidernow use the new 3-arg visitor constructors, passing the open buffer text so comment reattachment always sees exactly the document being formatted instead of re-reading theSourceUnit.nextflow lint -format, via a sharedFormattingProvider.commentsPreserved()helper built on the publicCommentReattacher.commentTexts: if formatting would remove or alter any comment, the provider shows an error and returns no edits.Code lens conversion (verified, no change needed)
convertScriptToTyped/convertProcessToTypedformat freshly-built AST nodes viavisitProcess/visitParamsdirectly;CommentReattacher.applyonly runs insidevisit(), so reattached comment metadata never touches the mutated ASTs and reattached leading comments cannot be duplicated into the replacement text. Pinned with a format-then-convert regression test.Tests
fmt: skip/fmt: off/fmt: on, include sorting, multi-line string re-indentation, and CRLF comment handling.maxLineLength=0disables wrapping, and fmt-directive regions round-trip unchanged.TestUtils.openOnDisk()/deleteOnDisk()helpers: tests issuing multiple requests against the same file must have it on disk, or the deferred workspace scan evicts the in-memory-only file from the AST cache.ConfigFormattingTestrebuilt on the sharedTestUtilsharness.Issue verification
The following issues are fixed by nextflow-io/nextflow#7346 and verified here with LS-level tests; GitHub does not auto-close cross-repo references, so they need manual closing once this ships: #26, #54, #75, #111, #115, #116, #127, #140, #150, #153.
Release ordering
build.gradlecommit here to the released versionmaxLineLength)Testing
Full suite: 271 tests, all passing except the pre-existing
PluginSpecCacheTestnetwork fetch, which fails identically onmainin the sandbox used for development (blocked network egress).🤖 Generated with Claude Code
https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb
Generated by Claude Code