-
Notifications
You must be signed in to change notification settings - Fork 769
Format options + fourslash testing + fix formatting bugs #2370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…o formatOptions
…o formatOptions
…o formatOptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds formatting options support and integrates it with the language server, enables fourslash testing for formatting functionality, and fixes a formatting bug related to extra spaces after newlines.
Key changes:
- Adds
FormatCodeSettingsstructure with comprehensive formatting options - Refactors configuration management to support both TypeScript and JavaScript scoped settings
- Fixes scanner bug that was calculating end-of-line positions incorrectly
- Updates fourslash test infrastructure to support formatting tests
- Reduces baseline diffs showing improved formatting convergence with TypeScript
Reviewed changes
Copilot reviewed 283 out of 283 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
internal/scanner/scanner.go |
Fixed off-by-one error in GetECMAEndLinePosition to correctly calculate line endings |
internal/project/configuration.go |
New configuration structure supporting separate TypeScript/JavaScript preferences |
internal/project/snapshot.go |
Refactored to use pointer-based Config and removed embedded format options |
internal/project/session.go |
Updated to manage Config objects instead of bare UserPreferences |
internal/ls/lsutil/formatcodeoptions.go |
New comprehensive formatting options with parsing and default values |
internal/ls/lsutil/userpreferences.go |
Integrated FormatCodeSettings into UserPreferences structure |
internal/ls/format.go |
Updated to use new format options from UserPreferences |
internal/lsp/server.go |
Enhanced configuration request to support multiple scopes (typescript/ts/javascript/js) |
| Test files | Multiple fourslash tests updated to use new verification methods |
| Baseline files | Reduced diff files showing formatting convergence improvements |
| Section: ptrTo("typescript"), | ||
| }, | ||
| { | ||
| Section: ptrTo("ts"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual section is ts/js, but we should just ignore those for now since we don't pull inferred project compiler options out of it yet.
…o formatOptions
…o formatOptions
Adds formatting options and links up the server's
FormatCodeOptionswith the formatting functions. Also adds in basic structure forjs,javascript,ts, andtypescriptscoped settings, and redirectsLanguageService.UserPreferences()to return the preferences for the active file.Moves
formatCodeSettingstolsutilFormatting is now being tested in fourslash, although not all the commands are implemented due to non-formatting tests crashing on insertion. I've added a fourslashTest property to skip the check for those specific crashes, so that the rest of the test can proceed as normal. The flag(s) can easily be commented out if investigating these crashes.
Also fixes a formatting bug that was adding extra spaces after newlines.