Skip to content

Conversation

@ndonfris
Copy link
Owner

@ndonfris ndonfris commented Nov 14, 2025

Refined Progress Reporting Behavior

This PR improves UX by disabling intrusive progress notifications during routine document edits while preserving them for meaningful background operations (server startup and workspace indexing).

Changes

  • Disabled progress reporting for textDocument/didChange events - no more popups when editing documents BUG: Unable to disable logger handler #121
  • Preserved progress reporting for server initialization and when go-to-definition opens documents outside the workspace
  • Fixed race condition in async progress token creation using a queue system. During the server.onInitialized() request that creates the progress reporter, a race condition is created by the await ProgressNotification.create('onInitialized') promise which only completes once the client initializes. This means our progress reporter could potentially not be connected to the client while it is receiving the progress.report() notifications, so store them in a queue. The queue is flushed when after the client confirms it has attached the progress reporter.
  • Improved diagnostics performance with direct connection.sendDiagnostic() calls and simplified caching (removed debouncing)
  • Added diagnostic code 7001 for "unknown command" warnings (commands outside of $PATH, or not reachable in the current document).
  • Added fish-lsp.showReferences command to open all unopened reference URIs
  • Implemented variadic parameter support for server's workspace/executeCommand handlers
  • Achieved 100% test coverage for src/utils/file-operations.ts
  • Added SyncFileHelper.expandNormalize() utility method
  • Improved fish-lsp info --time-startup to match normal server logs with proper $__fish_config_dir and $__fish_data_dir expansion
  • fish-lsp info --time-startup --use-workspace $__fish_config_dir behaves almost exactly the same as a normal client connection using the same workspace, stopping at after the indexing is complete. The following snippet below has proven useful for seeing order of connection methods used during startup.
    fish-lsp info --time-startup --only-workspace $__fish_config_dir --no-warning
    cat "$(fish-lsp info --log-file)"
  • Added new configuration: config.fish_lsp_show_client_popups (defaults to true)
  • Update config documentation to reflect changes towards env variables used by progress reporter
  • Disable config.fish_lsp_single_workspace_support variable's ability to control searching for symbols outside the workspace

Fixes #121
22 files changed: 2,180 insertions, 323 deletions
Commits: cc77cf3..7ae91c3 (10 commits)

@ndonfris ndonfris linked an issue Nov 14, 2025 that may be closed by this pull request
@ndonfris
Copy link
Owner Author

notification progress demo:
nl 122

@ndonfris ndonfris added this to the v1.1.1 milestone Nov 14, 2025
Bumps the npm_and_yarn group with 1 update in the / directory:
[vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite).

Updates `vite` from 7.1.5 to 7.1.11
- [Release notes](https://github.com/vitejs/vite/releases)
-
[Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md)
-
[Commits](https://github.com/vitejs/vite/commits/v7.1.11/packages/vite)

---
updated-dependencies:
- dependency-name: vite
  dependency-version: 7.1.11
  dependency-type: direct:development
  dependency-group: npm_and_yarn
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot]
<49699333+dependabot[bot]@users.noreply.github.com>
@ndonfris ndonfris force-pushed the feat/progress-notification-disabling branch from b135556 to 7e4b4e9 Compare November 18, 2025 04:05
ndonfris and others added 10 commits November 17, 2025 22:14
…lize()` + 100% coverage

- 100% test coverage for `src/utils/file-operations.ts` in
`tests/file-operations.test.ts`

- `expandNormalize()` method added to `SyncFileHelper` for expanding
paths and then performing `path.normalize(expanded)`

- updated `package.json` scripts `yarn test:*`/`yarn test` so that
`USER=test_user` inside `tests/*.test.ts`
… unknown command

- `src/utils/progress-notification.ts` was refactored to barebones
builder class for passing around & initializing `progress` reporter

- `7001` diagnostic code for warning user of `unknown command` was added

---
perf(src/server.ts): speed up server
- server now directly calls `connection.sendDiagnostic()` for displaying
  diagnostics in the client (displays much faster)
- `src/diagnostics/cache.ts` no longer debounces intervals, just
  directly caches diagnostics for the current document
- `server.didChangeTextDocument()` no longer shows any `progress`
  reporting. However, `server.didOpenTextDocument()` kept its progress
  reporting

---
docs(src/analyze.ts): added thorough ts-doc comments
- `src/snippets/fishlspEnvVariables.json` adds 7001 diagnostic to env
  variables
- other docs added like `src/diagnostics/error-codes.ts` for 7001

---
test: removed old diagnostic cache test cases
…ound process

- fix(src/config.ts): `config.fish_lsp_show_client_popups` defaults to
`true` now

- fix(src/utils/progress-notification.ts): allows for mutliple progress
instances to be specified at once while keeping track of which ones
belong to who

- fix(src/server.ts): Major changes to the `server.onInitialized()`
logic and behavior. Now correctly displays progress during server
startup process a single time, and correctly includes both
`progress.report(percentage, message)` in its output

- fix(src/utils/workspace-manager.ts): removed synchronous calls from
`WorkspaceManager.handleOpenDocument()` with commments
…rmal server logs

`fish-lsp info --time-startup` correctly expands both
`$__fish_config_dir` and `$__fish_data_dir` to their actual locations.

It also correctly treats the --time-startup operation like command:
```sh
nvim {$__fish_config_dir,$__fish_data_dir}/config.fish
```
would behave.

To 1:1 match the normal `fish-lsp start` logs, pass
`--use-workspace=$__fish_data_dir` to the `--time-startup` switch.

Matches server/logging behavior on branch:
feat/debounce-progress-notification
…ync token` creation

* we use a queue to store the `progress.begin()` `progress.report()`
  `progress.end()` requests that could be passed to the
  `async ProgressNotification()` that might not be initailized yet if
  the client starts up slow.

* the queue is flushed after the `async token` has succesfully made it
  back from the client, and meaning the client is succesfully initalized

* uses `connection.sendNotification()` instead of wrapper class the
  `vscode-languageserver` provides.

-----
@todo: chore(src/utils/workspace.ts): probably could be simplified in
-----       how it handles the single workspace mode
@ndonfris ndonfris force-pushed the feat/progress-notification-disabling branch from 7e4b4e9 to c8c6e4c Compare November 18, 2025 04:15
@ndonfris
Copy link
Owner Author

ndonfris commented Nov 18, 2025

branch testable on release: [email protected]

nightly install links:

nightly install command:

# npm
npm un fish-lsp
npm i -g [email protected]
fish-lsp info

# yarn
yarn global remove fish-lsp
yarn global add fish-lsp@nightly
fish-lsp info 

- `config.fish_lsp_single_workspace_support` variable now is used for
considering if our startup background analysis should be limited to the
current open workspace or `fish_lsp_all_indexed_workspaces`.

- `fish_lsp_single_workspace_support` previously limited definition
request fallback behavior by (if it was enabled) it would prevent the
server from trying to use fish child_process to resolve a command's
path. Now, fish_lsp_single_workspace_support does not effect this
fallback operation at all.
- `docs/CHANGELOG.md` for 1.1.1-pre.3

- removed `config.fish_lsp_semantic_handler_type` env variable
@ndonfris ndonfris force-pushed the feat/progress-notification-disabling branch from c8c6e4c to 9154c3b Compare November 18, 2025 06:22
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: Unable to disable logger handler

2 participants