The `FileLocation` and `PathOrId` types can borrow paths rather than
requiring them to be owned. This takes a refactor of the preview
functions and preview internals within `Picker`. With this change we
avoid an unnecessary `PathBuf` clone per render for any picker with a
file preview function (i.e. most pickers).
This refactor is not fully complete. The `PathOrId` is _sometimes_ an
owned `PathBuf`. This is for pragmatic reasons rather than technical
ones. We need a further refactor to introduce more core types like
`Location` in order to eliminate the Cow and only use `&Path`s within
`PathOrId`. This is left for future work as it will be a larger refactor
almost entirely fitting into the LSP commands module and helix-core -
i.e. mostly unrelated to refactoring the `Picker` code itself.
Co-authored-by: Pascal Kuthe <pascalkuthe@pm.me>
This introduces a custom URI type in core meant to be extended later
if we want to support other schemes. For now it's just a wrapper over a
PathBuf. We use this new URI type to firewall `lsp::Url`. This was
previously done in 8141a4a but using a custom URI type is more flexible
and will improve the way Pickers handle paths for previews in the child
commit(s).
Co-authored-by: soqb <cb.setho@gmail.com>
This fixes the changed files picker when used against a clean worktree
for example. Without it the running indicator does not disappear. It
also simplifies the dynamic query handler's implementation so that it
doesn't need to request a redraw explicitly.
Co-authored-by: Pascal Kuthe <pascalkuthe@pm.me>
We could expand on this in the future to have different preview modes
that you can toggle between with C-t. Currently that binding just hides
the preview but it could switch between different preview modes and in
one mode hide the path and just show the line contents.
The parent commit split out the workspace symbol picker to an inline
definition so the `workspace` parameter is never passed as `true`. We
should consolidate this picker definition into the symbol_picker
function.
DynamicPicker is a thin wrapper over Picker that holds some additional
state, similar to the old FilePicker type. Like with FilePicker, we want
to fold the two types together, having Picker optionally hold that
extra state.
The DynamicPicker is a little more complicated than FilePicker was
though - it holds a query callback and current query string in state and
provides some debounce for queries using the IdleTimeout event.
We can move all of that state and debounce logic into an AsyncHook
implementation, introduced here as `DynamicQueryHandler`. The hook
receives updates to the primary query and debounces those events so
that once a query has been idle for a short time (275ms) we re-run
the query.
A standard Picker created through `new` for example can be promoted into
a Dynamic picker by chaining the new `with_dynamic_query` function, very
similar to FilePicker's replacement `with_preview`.
The workspace symbol picker has been migrated to the new way of writing
dynamic pickers as an example. The child commit will promote global
search into a dynamic Picker as well.
We will use this in the child commit to improve the picker's running
indicator. Nucleo 0.4.0 includes an `active_injectors` member that we
can use to detect if anything can push to the picker. When that count
drops to zero we can remove the running indicator.
Nucleo 0.4.1 contains a fix for crashes with interactive global search
on a large directory.
This works nicely for dynamic pickers: we stop any running jobs like
global search that are pushing to the injector by incrementing the
version number when we start a new request. The boolean only allowed
us to shut the picker down once, but with a usize a picker can have
multiple "sessions" / "life-cycles" where it receives new options
from an injector.
Now that the picker is defined as a table, we need a way to provide
input for each field in the picker. We introduce a small query syntax
that supports multiple columns without being too verbose. Fields are
specified as `%field pattern`. The default column for a picker doesn't
need the `%field` prefix. The field name may be selected by a prefix
of the field, for example `%p foo.rs` rather than `%path foo.rs`.
Co-authored-by: ItsEthra <107059409+ItsEthra@users.noreply.github.com>
`menu::Item` is replaced with column configurations for each picker
which control how a column is displayed and whether it is passed to
nucleo for filtering. (This is used for dynamic pickers so that we can
filter those items with the dynamic picker callback rather than nucleo.)
The picker has a new lucene-like syntax that can be used to filter the
picker only on certain criteria. If a filter is not specified, the text
in the prompt applies to the picker's configured "primary" column.
Adding column configurations for each picker is left for the child
commit.
The picker previously used the IdleTimeout event as a trigger for
syntax-highlighting the currently selected document in the preview pane.
This is a bit ad-hoc now that the event system has landed and we can
refactor towards an AsyncHook (like those used for LSP completion and
signature-help). This should resolve some odd scenarios where the
preview did not highlight because of a race between the idle timeout
and items appearing in the picker.
This fixes the modification indicator when saving from insert mode with
a config such as
[keys.insert]
C-s = ":write"
Previously the modification indicator would be stuck showing modified
even if the buffer contents matched the disk contents when writing after
some changes in insert mode with this binding. In insert mode we do not
eagerly write undo checkpoints so that all changes made become one
checkpoint as you exit insert mode. When saving, `Document`s `changes`
`ChangeSet` would be non-empty and when there are changes we show the
buffer as modified. Then switching to normal mode would append the
changes to history, bumping the current revision past what it was when
last saved. Since the last saved revision and current revision were then
unsynced, the modification indicator would always show modified.
This matches [Kakoune's behavior]. Kakoune has a different architecture
for writes but a very similar system for history, transactions and undo
checkpoints (what it calls "undo groups"). Upon saving Kakoune creates
an undo checkpoint if there are any uncommitted changes. It does this
after the write has gone through since its writing system is different.
For our writing system it's cleaner to make the undo checkpoint before
performing the save so that the history revision increments before we
send the save event.
[Kakoune's behavior]: 80fcfebca8/src/buffer.cc (L565-L566)
Previously `gf` on `~/.config/helix` for example would error if the
entire path was selected but succeed and open a picker for the directory
contents if the selection was one one-width cursor. We need to expand
tildes for all paths instead of just the auto-detected paths.
This also refactors the `goto_file` blocks a little so that we construct
`paths` once instead of creating the Vec and immediately clearing it
when the selection is one single-width cursor.
This fix allows for multiple language servers at once which support
workspace commands. This was previously broken as just the first
language server supporting workspace commands was queried when listing
allowed worspace commands.
The fix is in two parts. Firstly, querying all workspace commands from
all language servers available and using them when actually running the
command in `lsp_workspace_command`. Secondly, doing the same in
`completers::lsp_workspace_command` such that completion still works as
expected.
The fix has one remaining issue, which I am unsure how to handle in the
best way possible, but which I also don't think should happen often:
Multiple language servers may register commands with the same name. This
will lead to that command being listed in the popup menu and in the
completion list multiple times, which can be possibly confusing. One
could disambigue them in the popover menu, but I am not sure the same
can be done for completion. When running `lsp-workspace-command` with
parameters, this behavior is "fixed" by displaying an error in that
case. I am unsure if this is the best fix for this issue in that case,
but could not find a better one.
Saving while in insert mode causes issues with the modification
indicator and this is very easy to reproduce with the current state of
the auto-save hook. We can tweak the hook slightly to await the mode
switch out of insert mode to perform the save.
The debounce is preserved: if you save and then immediately exit insert
mode the debounce will be respected. If the debounce lapses while you
are in insert mode, the save occurs as you switch out of insert mode
immediately.
* Implement check before adding path to files
* fix problem where directories were removed from args.files
* Revert "Implement check before adding path to files"
This reverts commit c123944d9b.
* Dissallow opening of irregular non-symlink files
* Fixed issue with creating new file from command line
* Fixed linting error.
* Optimized regularity check as suggested in review
* Created DocumentOpenError Sum Type to switch on in Application
* Forgot cargo fmt
* Update helix-term/src/application.rs
Accept suggestion in review.
Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
* Moved thiserror version configuration to the workspace instead of the individual packages.
---------
Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
* auto save after delay
* configable
* clearer names
* init
* working with some odd behaviour
* working with greater consistency
* Apply reviewer suggestions
- Remove unneccessary field
- Remove blocking save
* Improve auto-save configuration
Auto save can be configured to trigger on focus loss:
```toml
auto-save.focus-lost = true|false
```
and after a time delay (in milli seconds) since last keypress:
```toml
auto-save.after-delay.enable = true|false
auto-save.after-delay.timeout = [0, u64::MAX] # default: 3000
```
* Remove boilerplate and unnecessary types
* Remove more useless types
* Update docs for auto-save.after-delay
* Fix wording of (doc) comments relating to auto-save
* book: Move auto-save descriptions to separate section
---------
Co-authored-by: Miguel Perez <miguelvojito@gmail.com>
Co-authored-by: Miguel Perez <perezoji@cs.fsu.edu>
* Remove special-casing of line ending characters in selection replacement
* Refactor line ending handling and integration test to address code review comments
This is useful for resetting multiple changes at once. For example you
might use 'maf' or even '%' to select a larger region and reset all
changes within.
The original behavior of resetting the change on the current line is
retained when the primary selection is 1-width since we look for chunks
in the line range of each selection.
It can be convenient to define project specific debugger templates, some of
which might not necessitate prompting the user to define completion.
This commit makes completion optional for debugger templates and starts the
dap immediately if undefined or empty.
some LSPs does update the active signature and some not. To make both
worlds happy, make the active signature more intelligent.
1. SignatureHelp store now the suggested lsp_signature
2. if the lsp_signature changes then use it
3. otherwise use the last signature from the old popup
4. in case the old signature doesn't exist anymore, show the last signature
Signed-off-by: Ben Fekih, Hichem <hichem.f@live.de>
Trunctation should always be handled by the parent. Returning None is
only supposed to indicate a missing implementation
Co-authored-by: Ben Fekih, Hichem" <hichem.f@live.de>
While moving completion resolve to the event system in #9668 we introduced what
is essentially a "DOS attack" on slow LSPs. Completion resolve requests were
made in the render loop and debounced with a timeout. Once the timeout expired
the resolve request was made. The problem is the next frame would immediately
request a new completion resolve request (and mark the old one as obsolete but
because LSP has no notion of cancelation the server would still process it). So
we were in essence sending one completion request to the server every 150ms and
only stopped if the server managed to respond before we rendered a new frame.
This caused overload on slower machines/with slower LS.
In this PR I revamped the resolve handler so that a request is only ever
resolved once. Both by checking if a request is already in-flight and by marking
failed resolve requests as resolved.
Make the popup positions more consistent.
Improvements:
1. if the signature popup content is bigger than the available space,
then the popup is always shown under the cursor, even if there more
space above the cursor than below
2. There is no mutation anymore inside required_size. Maybe in the future
we can update all widgets to have no mutations and change the trait
Signed-off-by: Ben Fekih, Hichem <hichem.f@live.de>
Currently the editor mode has no effect on the behavior of `search` and
`rsearch`. We can pass in the right movement for the editor mode to make
the behavior or `search` and `rsearch` match `search_next` and
`search_prev` in select mode.
implement handle_event to cycle through the function signatures.
To change the signature press alt+p/n .
Signed-off-by: Ben Fekih, Hichem <hichem.f@live.de>
when the available height for the popup is low/small, then it is not
possible to scroll until the end
Signed-off-by: Ben Fekih, Hichem <hichem.f@live.de>
Previously unnecessary/deprecated diagnostic tags replaced the highlight
for the severity of a diagnostic. This could cause either the severity
or unnecessary/deprecated scopes to disappear when diagnostic ranges
overlapped though. Plus the severity highlight can be interesting in
addition to the unnecessary/deprecated highlight.
So this change separates the unnecessary and deprecated highlights from
the severity highlights, so each is merged separately and when they
overlap, the highlights are combined.
Prior to this change, every integration test which wanted its line
endings to be handled transparently across platforms, i.e. test with
the same input that has its platform's line feed characters, converting
the line endings was up to each individual test by calling the
`platform_line` helper function. This significantly increases the amount
of boilerplate one has to copy between all the tests.
However, there are some test cases that need to exert strict control
over the exact input text without being manipulated behind the scenes by
the test framework.
So, with this change, the line feed conversions are factored into
the `TestCase` struct. By default, line endings of the input text
are converted to the platform's native line feed ending, but one can
explicitly specify in their test case when the input text should be left
alone and tested as is.
This reverts commit 0dc67ff885.
See the post-merge discussion in #9828. The old behavior was less
surprising and we have other ways to abort from a prompt, so let's
revert the behavior change.
This uses the new TreeCursor type from the parent commit to reimplement
the tree-sitter motions (`A-p/o/i/n`). Other tree-sitter related
features like textobjects are not touched with this change and will
need a different, unrelated approach to solve.
The refactor in bcf7b263 introduced a possible subtraction with overflow
when the statusline is layed out so that the left or right sides are
larger than the padding it would take to align the center area to the
middle.
When the left or right areas are too large, we can evenly space the
elements rather than trying to align the center area to the middle.
This prevents possible underflows and makes sense visually - it's
still easy to tell the areas apart at a glance.
