This matches the insert-mode behavior for Vim and Kakoune: if the
current line is empty except for whitespace, `<ret>` should insert a
line ending at the beginning of the line, moving any indentation to the
next line.
The 'revisions' field on History can't be treated as linear: each
Revision in the revisions Vec has a parent link and an optional child
link. We can follow those to unroll the recent history.
When using undo/redo, the history revision can be decremented. In that
case we should apply the inversions since the given revision in
History::changes_since. This prevents panics with jumplist operations
when a session uses undo/redo to move the jumplist selection outside
of the document.
This case panics since undo/redo call View::apply and here, the edit
that moves the jumplist selection out-of-bounds is not yet applied when
View::apply is called in undo/redo. View::apply should only be called
by the EditorView now.
* Add a test case for updating jumplists across windows
* Apply transactions to all views on history changes
This ensures that jumplist selections follow changes in documents, even
when there are multiple views (for example a split where both windows
edit the same document).
* Leave TODOs for cleaning up View::apply
* Use Iterator::reduce to compose history transactions
Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>
Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>
Language Servers may signal that they do not support a method in
the initialization result (server capabilities). We can check these
when making LSP requests and hint in the status line when a method
is not supported by the server. This can also prevent crashes in
servers which assume that clients do not send requests for methods
which are disabled in the server capabilities.
There is an existing pattern the LSP client module where a method
returns `Option<impl Future<Output = Result<_>>>` with `None` signaling
no support in the server. This change extends this pattern to the rest
of the client functions. And we log an error to the statusline for
manually triggered LSP calls which return `None`.
Previously, jumplists could grow unchecked. Every transaction is
applied to jumplist selections to ensure that they are up to date
and within document bounds, so this would cause every edit to become
more expensive as jumplist lengths increased throughout a session.
Setting a maximum number of entries limits the cost.
Vim and Neovim limit their jumplists:
* b298fe6cba/src/structs.h (L141)
* e8cc489acc/src/nvim/mark_defs.h (L57)
Notably, Kakoune does not. In Kakoune, changes are applied to jumplist
entries lazily as you hit `<C-o>`/`<C-i>` though, so Kakoune doesn't
have the same growing cost concerns. Kakoune also does not have a
concept of a View which limits the cost further.
Vim and Neovim limit to 100. This seems unreasonably high to me so I've
set this to 30 to start. We can increase if this is problematically
low.
d6323b7cbc changed the behavior of paste
to select the newly inserted text. This is preferrable in normal mode
because it's useful to be able to act on the new text. This behavior
is worse for insert or select mode though:
* In insert mode, the cursor ends up on the last character of the newly
selected text, so further typing inserts text before the last
character.
* In select mode, the current selection is replaced with the new text
selection which doesn't extend the current selection. With this
change, the selection is extended to include the new text.
This aligns the behavior more closely with Kakoune, but it's
coincidental instead of intentional: Kakoune doesn't implement
bracketed paste (AFAIK) which causes this behavior in insert mode,
and Kakoune doesn't have a select mode.
Previously, commands such as `r<tab>` (replace with tab) or `t<tab>`
(select till tab) had no effect. This is because `KeyCode::Tab` needs
special treatment (like `KeyCode::Enter`).
This change handles a language server exiting. This was a UX sore-spot:
if a language server crashed, Helix did not recognize the exit and
continued to send requests to it. All requests would timeout since they
would not receive responses. This would also hold-up Helix closing
itself down since it would try to gracefully shutdown the server which
is implemented in the LSP spec as a request.
We could attempt to automatically restart the language server on crash.
I left this for future work since that change will need to be slightly
complicated: it will need to cover the case of a language server
repeatedly crashing.
d7d0d5ffb7 resolves completion items on
the idle-timeout event. The `Completion::resolve_completion_item`
function blocks on the LSP request though, which blocks the compositor
and in turn blocks the event loop. So until the language server returns
the resolved completion item, Helix is unable to respond to keypresses
or other LSP messages.
This is typically ok since the resolution request is fast but for some
language servers this can be problematic, and ideally we shouldn't be
blocking like this anyways.
When receiving a `completionItem/resolve` request, the Volar server
sends a `workspace/configuration` request to Helix and blocks itself
on the response, leading to a deadlock. Eventually the resolve request
times out within Helix but Helix is locked up and unresponsive in that
window.
This change resolves the completion item without blocking the
compositor.
PR #4134 switched the autocomplete menu from alphabetical to fuzzy
sorting. This commit removes the still existing filtering by prefix and
should enable full fuzzy sorting of the autocomplete menu.
closes#3084, #1807
Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>
This fixes an edge case for completing shellwords. With a file
"a b.txt" in the current directory, the sequence `:open a\<tab>`
will result in the prompt containing `:open aa\ b.txt`. This is
because the length of the input which is trimmed when replacing with
completion is calculated on the part of the input which is parsed by
shellwords and then escaped (in a separate operation), which is lossy.
In this case it loses the trailing backslash.
The fix provided here refactors shellwords to track both the _words_
(shellwords with quotes and escapes resolved) and the _parts_ (chunks
of the input which turned into each word, with separating whitespace
removed). When calculating how much of the input to delete when
replacing with the completion item, we now use the length of the last
part.
This also allows us to eliminate the duplicate work done in the
`ends_with_whitespace` check.
The text within the command palette used a custom format to display
the keybinding for a command. This change switches to the key sequence
format that we use for pending keys and macros.
* init
* cargo fmt
* optimisation of the scrollbar render both for Menu and Popup. Toggling off scrollbar for Popup<Menu>, since Menu has its own
* rendering scroll track
* removed unnecessary cast
* improve memory allocation
* small correction
d6323b7cbc introduced a regression for
shell commands like `|`, `!`, and `<A-!>` which caused the new
selections to be incorrect. This caused a panic when piping (`|`)
would cause the new range to extend past the document end.
The paste version of this bug was fixed in
48a3965ab4.
This change also inherits the direction of the new range from the old
range and adds integration tests to ensure that the behavior isn't
broken in the future.
* dynamically resize line number gutter width
* removing digits lower-bound, permitting spacer
* removing max line num char limit; adding notes; qualified successors; notes
* updating tests to use new line number width when testing views
* linenr width based on document line count
* using min width of 2 so line numbers relative is useful
* lint rolling; removing unnecessary type parameter lifetime
* merge change resolution
* reformat code
* rename row_styler to style; add int_log resource
* adding spacer to gutters default; updating book config entry
* adding view.inner_height(), swap for loop for iterator
* reverting change of current! to view! now that doc is not needed
If `a\ b.txt` were a local file, `:o a\ <tab>` would fill the prompt
with `:o aa\ b.txt` because the replacement range was calculated using
the shellwords-parsed part. Escaping the part before calculating its
length fixes this edge-case.
This changes the completion items to be rendered with shellword
escaping, so a file `a b.txt` is rendered as `a\ b.txt` which matches
how it should be inputted.
8584b38cfb switched to shellwords for
completion in command-mode. This changes the conditions for choosing
whether to complete the command or use the command's completer.
This change processes the input as shellwords up-front and uses
shellword logic about whitespace to determine whether the command
or argument should be completed.
* Fix range offsets in multi-selection paste
d6323b7cbc introduced a regression with
multi-selection paste where pasting would not adjust the ranges
correctly. To fix it, we need to track the total number of characters
inserted in each changed selection and use that offset to slide each
new range forwards.
* Inherit selection directions on paste
* Add an integration-test for multi-selection pasting
The sequence "_y"_p panics because the blackhole register contains an
empty values vec. This causes a panic when pasting since it unwraps
a `slice::last`.
This follows changes in Kakoune to the same effects:
* p/<space>p: 266d1c37d0
* !/<A-!>: 85b78dda2e
Selecting the new data inserted by shell or pasting is often more
useful than retaining a selection of the pre-paste/insert content.
* Clamp highlighting range to be within document
This fixes a panic possible when two vsplits of the same document
exist and enough lines are deleted from the document so that one of
the windows focuses past the end of the document.
* Ensure cursor is in view on window change
If two windows are editing the same document, one may delete enough of
the document so that the other window is pointing at a blank page (past
the document end). In this change we ensure that the cursor is within
view whenever we switch to a new window (for example with `<C-w>w`).
* Update helix-term/src/ui/editor.rs
Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>
Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>
When backward-deleting a character, if this character and the following
character form a Pair, we want to delete both. However, there is a bug
that deletes both characters also if both characters are closers of some
Pair.
This commit fixes that by adding an additional check that the deleted
character should be an opener in a Pair.
Closes https://github.com/helix-editor/helix/issues/4544.
Most commands that accept an argument show their current value if no
argument is specified. The `:theme` command previously displayed an
error message in the status bar if not provided with an argument:
```
Theme name not provided
```
It now shows the current theme name in the status bar if no argument is
specified.
Signed-off-by: James O. D. Hunt <jamesodhunt@gmail.com>
Signed-off-by: James O. D. Hunt <jamesodhunt@gmail.com>
This bug occurs on `shell_insert_output` and `shell_append_output`
commands.
The previous implementation would create a child process using the Rust
stdlib's `Command` builder. However, when nothing should be piped in
from the editor, the default value for `stdin` would be used. According
to the Rust stdlib documentation that is `Stdio::inherit` which will
make the child process inherit the parent process' stdin. This would
cause the terminal to freeze.
This change will set the child process' stdin to `Stdio::null` whenever
it doesn't pipe it. In the `if` statement where this change was made
there was an extra condition for windows that I am not sure if would
require some special treatment.
This is mostly for the sake of the diagnostics pickers: without
rendering the diagnostic styles, it's hard to tell where the entries
in the picker are pointing to.
Some language servers may not send the `documentation` field if it
is expensive to compute. Clients can request the missing field with
a completionItem/resolve request.
In this change we use the idle-timeout event to ensure that the current
completion item is resolved.
This complicates the code a little but it often divides by two the number of allocations done by
the functions. LSP labels especially can easily be called dozens of time in a single menu popup,
when listing references for example.
When we do auto formatting, the code that takes the LSP's response and applies
the changes to the document are just getting the currently focused view and
giving that to the function, basically always assuming that the document that
we're applying the change to is in focus, and not in a background view.
This is usually fine for a single view, even if it's a buffer in the
background, because it's still the same view and the selection will get updated
accordingly for when you switch back to it. But it's obviously a problem for
when there are multiple views, because if you don't have the target document in
focus, it will ask the document to update the wrong view, hence the crash.
The problem with this is picking which view to apply any selection change to.
In the absence of any more data points on the views themselves, we simply pick
the first view associated with the document we are saving.
When force quitting, we need to block on the pending writes to ensure
that write commands succeed before exiting, and also to avoid a crash
when all the views are gone before the auto format call returns from
the LS.
* Autosave all when the terminal loses focus
* Correct comment on focus config
Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>
* Need a block_try_flush_writes in all quit_all paths
Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>
* Fix test::print for Unicode
The print function was not generating correct translations when
the input has Unicode (non-ASCII) in it. This is due to its use of
String::len, which gives the length in bytes, not chars.
* Fix multi-code point auto pairs
The current code for auto pairs is counting offsets by summing the
length of the open and closing chars with char::len_utf8. Unfortunately,
this gives back bytes, and the offset needs to be in chars.
Additionally, it was discovered that there was a preexisting bug where
the selection was not computed correctly in the case that the cursor
was:
1. a single grapheme in width
2. this grapheme was more than one char
3. the direction of the cursor is backwards
4. a secondary range
In this case, the offset was not being added into the anchor. This was
fixed.
* migrate auto pairs tests to integration
* review comments
This change removes language server configuration from the default
languages.toml config for integration tests. No integration-tests
currently depend on the availability of a language server but if any
future test needs to, it may provide a language server configuration
by passing an override into the `test_syntax_conf` helper.
Language-servers in integration tests cause false-positive failures
when running integration tests in GitHub Actions CI. The Windows
runner appears to have `clangd` installed and all OS runners have
the `R` binary installed but not the `R` language server package.
If a test file created by `tempfile::NamedTempFile` happens to have a
file extension of `r`, the test will most likely fail because the
R language server will fail to start and will become a broken pipe,
meaning that it will fail to shutdown within the timeout, causing a
false-positive failure. This happens surprisingly often in practice.
Language servers (especially rust-analyzer) also emit unnecessary
log output when initializing, which this change silences.
`helix_view::apply_transaction` closes over `Document::apply` and
`View::apply` to ensure that jumplist entries are updated when a
document changes from a transaction. `Document::apply` shouldn't
be called directly - this helper function should be used instead.
If a document is written with a new path, currently, in the event that
the write fails, the document still gets its path changed. This fixes
it so that the path is not updated unless the write succeeds.
The way that document writes are handled are by submitting them to the
async job pool, which are all executed opportunistically out of order. It
was discovered that this can lead to write inconsistencies when there
are multiple writes to the same file in quick succession.
This seeks to fix this problem by removing document writes from the
general pool of jobs and into its own specialized event. Now when a
user submits a write with one of the write commands, a request is simply
queued up in a new mpsc channel that each Document makes to handle its own
writes. This way, if multiple writes are submitted on the same document,
they are executed in order, while still allowing concurrent writes for
different documents.
Instead of repeatedly checking if it is in_bounds, calculate the
max_indent beforehand and just loop. I added a debug_assert to "prove"
that it never tries drawing out of bounds.
Better performance, and otherwise very long lines with lots of tabs
will wrap around the u16 and come back on the other side, messing up
the beginning skip_levels.
Also changes workspace diagnostic picker bindings to <space>D and
changes the debug menu keybind to <space>g, the previous diagnostic
picker keybind. This brings the diagnostic picker bindings more in
line with the jump to next/previous diagnostic bindings which are
currently on ]d and [d.
The debug assertion that document diagnostics are sorted incorrectly
panics for cases like `[161..164, 162..162]`. The merging behavior
in the following lines that relies on the assertion only needs the
input ranges to be sorted by `range.start`, so this change simplifies
the assertion to only catch violations of that assumption.
Undo/redo/earlier/later call `Document::apply_impl` which applies
transactions to the document. These transactions also need to be
applied to the view as in 0aedef0.
Here we separate the diagnostics by severity and then overlay the Vec
of spans for each severity on top of the highlights. The error
diagnostics end up overlaid on the warning diagnostics, which are
overlaid on the hints, overlaid on info, overlaid on any other severity
(default), then overlaid on the syntax highlights.
This fixes two things:
* Error diagnostics are now always visible when overlapped with other
diagnostics.
* Ghost text is eliminated.
* Ghost text was caused by duplicate diagnostics at the EOF:
overlaps within the merged `Vec<(usize, Range<usize>)>` violate
assumptions in `helix_core::syntax::Merge`.
* When we push a new range, we check it against the last range and
merge the two if they overlap. This is safe because they both
have the same severity and therefore highlight.
The actual merge is skipped for any of these when they are empty, so
this is very fast in practice. For some data, I threw together an FPS
counter which renders as fast as possible and logs the renders per
second.
With no diagnostics, I see an FPS gain from this change from 868 FPS
to 878 (+1.1%) on a release build on a Rust file. On an Erlang file
with 12 error diagnostics and 6 warnings in view (233 errors and 66
warnings total), I see a decrease in average FPS from 795 to 790
(-0.6%) on a release build.
It is easy to forget to call `Document::apply` and/or `View::apply` in
the correct order. This commit introduces a helper function which
closes over both calls.
This change adds View::apply calls for all Document::apply call-sites,
ensuring that changes to a document do not leave invalid entries in
the View's jumplist.
* Implement cursorcolumn
* Add documentation
* Separate column style from line with fallback
* Fallback to cursorcolumn first
* Switch to non-fallback try_get_exact
Add new function `try_get_exact`, which doesn't perform fallback,
and use that instead because the fallback behaviour is being handled
manually.
If the close method fails, the editor will quit before restoring the
terminal. This causes the shell to break if, e.g. the LS times out
shutting down.
This fixes this by always restoring the terminal after closing, and
printing out a message to stderr if there is an error.
This change automatically tracks pending text for for commands which use
on-next-key callbacks. For example, `t` will await the next key event
and "t" will be shown in the bottom right-hand corner to show that we're
in a pending state.
Previously, the text for these on-next-key commands needed to be
hard-coded into the command definition which had some drawbacks:
* It was easy to forget to write and clear the pending text.
* If a command was remapped in a custom config, the pending text would
still show the old key.
With this change, pending text is automatically tracked based on the
key events that lead to the command being executed. This works even
when the command is remapped in config and when the on-next-key
callback is nested under some key sequence (for example `mi`).
* Keep arrow and special keys in insert
Advanced users won't need it and is useful for beginners.
Revert part of #3671.
* Change text for insert mode section
Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>
* Remove ctrl-up/down in insert
* Reorganize insert keys and docs
* Improve page up experience on last tutor
The last tutor page can page down multiple times and it will break the
heading on the 80x24 screen paging when reaching the last page, this
keeps the style the same and make sure page up and down won't break it.
Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>
* Change focus to modified docs on quit
When quitting with modified documents, automatically switch focus to
one of them.
* Update helix-term/src/commands/typed.rs
Co-authored-by: Poliorcetics <poliorcetics@users.noreply.github.com>
* Make it work with buffer-close-all and the like
* Cleanup
Use Cow instead of String, and rename DoesntExist -> DoesNotExist
Co-authored-by: Poliorcetics <poliorcetics@users.noreply.github.com>
* Add option to skip the first indent guide
* reorder skip_first option
* change indent-guides.skip_first to a number
* rename skip -> skip_levels
* add skip_levels to the book
* Update book/src/configuration.md
Co-authored-by: A-Walrus <58790821+A-Walrus@users.noreply.github.com>
* Update helix-term/src/ui/editor.rs
Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
Co-authored-by: Robin <robinvandijk@klippa.com>
Co-authored-by: A-Walrus <58790821+A-Walrus@users.noreply.github.com>
Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
* keymap: Rename A "Insert at end of line"
The language for the `A` binding is potentially confusing because
`A` behaves like `i` done at the end of the line rather than `a`.
This change renames the command to match Kakoune's language[^1].
[^1]: 021da117cf/src/normal.cc (L2229)
* keymap: Rename I `insert_at_line_start`
* Select inserted space after join
* Split join_selections with space selection to A-J
Kakoune does that too and some users may still want to retain their selections.
* Update join_selections docs
When signature help is too large it may cause a panic when it is too
large, now I just make the hover do an intersection with surface to make
sure it never overflow.
This changes the behavior of operations like `]f`/`[f` to set the
direction of the new range to the direction of the action.
The original behavior was to always use the head of the next function.
This is inconsistent with the behavior of goto_next_paragraph and makes
it impossible to create extend variants of the textobject motions.
This causes a behavior change when there are nested functions. The
behavior in the parent commit is that repeated uses of `]f` will
select every function in the file even if nested. With this commit,
functions are skipped.
It's notable that it's possible to emulate the original behavior by
using the `ensure_selections_forward` (A-:) command between invocations
of `]f`.
* Show "Invalid regex" message on enter (Validate)
* Reset selection on invalid regex
* Add popup for invalid regex
* Replace set_position with position
* Make popup auto close
* Split helix_core::find_root and helix_loader::find_local_config_dirs
The documentation of find_root described the following priority for
detecting a project root:
- Top-most folder containing a root marker in current git repository
- Git repository root if no marker detected
- Top-most folder containing a root marker if not git repository detected
- Current working directory as fallback
The commit contained in https://github.com/helix-editor/helix/pull/1249
extracted and changed the implementation of find_root in find_root_impl,
actually reversing its result order (since that is the order that made
sense for the local configuration merge, from innermost to outermost
ancestors).
Since the two uses of find_root_impl have different requirements (and
it's not a matter of reversing the order of results since, e.g., the top
repository dir should be used by find_root only if there's not marker in
other dirs), this PR splits the two implementations in two different
specialized functions.
In doing so, find_root_impl is removed and the implementation is moved
back in find_root, moving it closer to the documented behaviour thus
making it easier to verify it's actually correct
* helix-core: remove Option from find_root return type
It always returns some result, so Option is not needed
* Improve keymap errors from command typos
Currently, opening helix with a config containing a bad command mapping
fails with a cryptic error. For example, say we have a config (bad.toml)
with a command name that doesn't exist:
[keys.normal]
b = "buffer_close" # should be ":buffer-close"
When we `hx -c bad.toml`, we get...
> Bad config: data did not match any variant of untagged enum KeyTrie for key `keys.normal` at line 1 column 1
> Press <ENTER> to continue with default config
This is because of the way that Serde tries to deserialize untagged
enums such as `helix_term::keymap::KeyTrie`. From the Serde docs[^1]:
> Serde will try to match the data against each variant in order and the
> first one that deserializes successfully is the one returned.
`MappableCommand::deserialize` fails (returns an Err variant) when a
command does not exist. Serde interprets this as the `KeyTrie::Leaf`
variant failing to match and declares that the input data doesn't
"match any variant of untagged enum KeyTrie."
Luckily the variants of KeyTrie are orthogonal in structure: we can tell
them apart by the type hints from a `serde:🇩🇪:Visitor`. This change
uses a custom Deserialize implementation along with a Visitor that
discerns which variant of the KeyTrie applies. With this change, the
above failure becomes:
> Bad config: No command named 'buffer_close' for key `keys.normal.b` at line 2 column 5
> Press <ENTER> to continue with default config
We also provide more explicit information about the expectations on
the field. A config with an unexpected type produces a message with
that information and the expectation:
[keys.normal]
b = 1
> Bad config: invalid type: integer `1`, expected a command, list of commands, or sub-keymap for key `keys.normal.b` at line 2 column 5
> Press <ENTER> to continue with default config
[^1]: https://serde.rs/enum-representations.html#untagged
* Update helix-term/src/keymap.rs
Co-authored-by: Ivan Tham <pickfire@riseup.net>
Co-authored-by: Ivan Tham <pickfire@riseup.net>
* Add command line parameter to specify log file
I had the logs of my debug helix mixed in with the logs from the
production helix.
Add a `--log` command line argument to redirect any logs to other
files, making my debugging easier :-)
* Update completion files with `--log` argument
The tutor file is loaded as .txt which can potentially spawn a
language server. Then the path is unset, but the LS remains active.
This can cause panics since updates are now submitted for a doc
with no path.
As a quick workaround we remove the extension which should avoid
detection.
Fixes#3730
* Don't change config to default when refreshing invalid config
* Propely handle theme errors with config-reload
* Extract refresh theme into seperate function
Helix is first and foremost a modal editor. Willingness to support non-modal
editing is there, but it is not one that should be encouraged with the default
settings. There are an increasing number of users who are stumbling because
they are trying to use Helix as a non-modal editor, so this is an effort to
encourage new users to stop and take notice that Helix has a different paradigm
than VSCode, Sublime, etc. Users can still add these bindings back to their own
configs if they wish.
`extend_line_above` (and `extend_line` when facing backwards) skip
a line when the current range does not fully cover a line.
Before this change:
foo
b#[|a]#r
baz
With `extend_line_above` or `extend_line` selected the line above.
#[|foo
bar]#
baz
Which is inconsistent with `extend_line_below`. This commit changes
the behavior to select the current line when it is not already
selected.
foo
#[|bar]#
baz
Then further calls of `extend_line_above` extend the selection up
line-wise.
