Skip to content
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

vim: Support = and ! #12565

Open
1 task done
ethanmsl opened this issue Jun 1, 2024 · 3 comments
Open
1 task done

vim: Support = and ! #12565

ethanmsl opened this issue Jun 1, 2024 · 3 comments
Labels
bug [core label] vim

Comments

@ethanmsl
Copy link

ethanmsl commented Jun 1, 2024

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

I was just perusing the vim cheat sheet linked in the Zed docs, directly after mentioning that zed vim is tested against Neovim.

Playing with some operators I noticed that many of them do not always work in Zed, but do work in Neovim.

In particular

  • gU - DONE
  • gu - DONE
  • < - DONE
  • > - DONE
  • =
  • !
    do NOT work in Normal mode.

The non-action is the same for all the above listed.

Some of them do work in Visual mode, but not in Normal mode.

Environment

Zed: v0.137.6 (Zed)
OS: macOS 14.5.0
Memory: 96 GiB
Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

examples:

Neovim

see the |readme ~~ gUw~~> see the README|
|let Some(thing) ~~ >w ~~> | let Some(thing)

Zed

see the |readme ~~ gUw~~> see the readme|
|let Some(thing) ~~ >w ~~> let |Some(thing)

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

No response

@ethanmsl ethanmsl added admin read Pending admin review bug [core label] triage Maintainer needs to classify the issue labels Jun 1, 2024
@ConradIrwin
Copy link
Member

@ethanmsl Thanks for reporting!

If you'd like to pair with me on implementing any of these, feel free to book time here: https://calendly.com/conradirwin/pairing.

Most of these are supported by the editor, but need a bit of wrapping to make them work vimilly (supporting motions, counts, and repeats, etc.) so I don't think it's too hard, but it does need doing.

I'm not sure what to do about ! yet, we do now have the infrastructure in place to spawn tasks, but I don't think we have anything else that reads the output back again, though I guess we can do that with some redirections.

@ConradIrwin ConradIrwin added vim and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Jun 3, 2024
@ethanmsl
Copy link
Author

ethanmsl commented Jun 3, 2024

I'd love to.
I placed a spot on your calendar Friday late morning (Jun 7th (F), 10am PST).
Feel free to punt it to next week or request a last minute reschedule. (I'll put it down as 'hack on something' time on my calendar, so nbd to change focus.)

ConradIrwin added a commit that referenced this issue Jun 7, 2024
Co-Authored-By: ethanmsl@gmail.com

Release Notes:

- vim: Added `gu`/`gU`/`g~` for changing case. (#12565)
@ConradIrwin ConradIrwin changed the title some Vim Operators do not function in Normal mode vim: Support = and ! Jun 7, 2024
@ConradIrwin
Copy link
Member

Updated issue to be clear about the current state of things!

Thanks for pairing on guU~ today, and I look forward to seeing what you come up with on !.

tomoikey added a commit to tomoikey/zed that referenced this issue Jun 11, 2024
.
support overload

delete unneccesary diff

delete unneccesary diff

add todo comment

add GetSignatureHelp to proto

.

.

.

Support markdown

.

hide popover on click esc

.

Add basic Wikipedia support to `/fetch` (zed-industries#12777)

This PR extends the `/fetch` slash command with the initial support for
Wikipedia's HTML structure.

Release Notes:

- N/A

Handle Wikipedia code blocks in `/fetch` command (zed-industries#12780)

This PR extends the `/fetch` command with support for Wikipedia code
blocks.

Release Notes:

- N/A

Check validity of new.range too (zed-industries#12781)

I'm not certain yet how it could be invalid, but we are still seeing
panics here.

Release Notes:

- Fixed a panic when opening the diagnostics view

Block publishing of `zed_extension_api` v0.0.7 (zed-industries#12784)

This PR adds a temporary block on publishing v0.0.7 of the
`zed_extension_api`.

We have breaking changes to the extension API that are currently staged
on `main` and are still being iterated on, so we don't want to publish
again until we're ready to commit to the new API.

This change is intended to prevent accidental publishing of the crate
before we're ready.

Release Notes:

- N/A

Add event for yarn project identification (zed-industries#12785)

Report a `open yarn project` `app_event` for each worktree where
`yarn.lock` is found and only report it once per session.

Release Notes:

- N/A

vim: Add gU/gu/g~ (zed-industries#12782)

Co-Authored-By: ethanmsl@gmail.com

Release Notes:

- vim: Added `gu`/`gU`/`g~` for changing case. (zed-industries#12565)

Use the new assistant icon in the setup instructions (zed-industries#12787)

This is a PR with just a small visual adjustment, so instructions are
up-to-date with the new icon.
I did not remove the "old" ai.svg as I am not sure if its gonna be used
in the future or if its has been completely replaced by the new "zed
assistant" icon.

Release Notes:

- Fixed the wrong icon being used in the assistant setup instructions.

For open ai

<img width="543" alt="image"
src="https://github.com/zed-industries/zed/assets/61624214/5f18a8f4-6761-4df5-8482-92582545dee5">

and anthropic

<img width="544" alt="image"
src="https://github.com/zed-industries/zed/assets/61624214/6ca3ed23-0f68-4c0d-bc8a-32ab7c607029">

how it looked before (Zed Preview 0.139.3
0c083b7):

<img width="526" alt="image"
src="https://github.com/zed-industries/zed/assets/61624214/af9c9fa8-89ed-4f6a-88ca-b285b4c522c3">

Refactor: Make it possible to share a remote worktree (zed-industries#12775)

This PR is an internal refactor in preparation for remote editing. It
restructures the public interface of `Worktree`, reducing the number of
call sites that assume that a worktree is local or remote.

* The Project no longer calls `worktree.as_local_mut().unwrap()` in code
paths related to basic file operations
* Fewer code paths in the app rely on the worktree's `LocalSnapshot`
* Worktree-related RPC message handling is more fully encapsulated by
the `Worktree` type.

to do:
* [x] file manipulation operations
* [x] sending worktree updates when sharing

for later
* opening buffers
* updating open buffers upon worktree changes

Release Notes:

- N/A

vim: add guu gUU g~~ g/ (zed-industries#12789)

Release Notes:

- vim: Add `g/` for project search

Rename `workspace::Restart` action into `workspace::Reload` (zed-industries#12672)

Closes zed-industries#12609

Instead of adding some ordering mechanism to the actions, rename the
action so that it's not interfering with the `editor: restart language
server` command.

Before:

![image](https://github.com/zed-industries/zed/assets/2690773/b5e86eda-d766-49fc-a25b-f8b9fdb7b521)

![image](https://github.com/zed-industries/zed/assets/2690773/c5edeb56-12aa-496b-bb6f-dc705cbb9ae3)

After:

![image](https://github.com/zed-industries/zed/assets/2690773/ed30c68d-bfdd-4e00-bb5d-0be52fbe4e16)
![Screenshot 2024-06-05 at 09 46
25](https://github.com/zed-industries/zed/assets/2690773/9fe4eb52-0399-4321-85a9-3b07c11395ce)

Release Notes:

- Improved language server restart command ergonomics by renaming
`workspace::Restart` action into `workspace::Reload` to remove any other
"restart"-worded actions in the list

cpp: Highlight sized type specifiers as keywords (zed-industries#12751)

Without this, `unsigned` or `unsigned int` is not highlighted properly:
`int` is a primitive_type but `unsigned` is a sized_type_specifier. This
is already handled in C as both are part of @type highlight group.

Before:

![image](https://github.com/zed-industries/zed/assets/1106629/7210b769-9dff-428c-9e4f-55b652f91674)

After:

![image](https://github.com/zed-industries/zed/assets/1106629/8661c412-30f0-4b44-a4a2-1860a0b56a4e)

Release Notes:

- N/A

Add auto-completion support for `package.json` files (zed-industries#12792)

![截屏2024-06-08 07 56
41](https://github.com/zed-industries/zed/assets/21101490/da97e7d4-458b-4262-ac23-a4704af4f015)

Release Notes:

- Added auto-completion support for `package.json` files.

linux: run runnables only when event loop is idle (zed-industries#12839)

This change ensures that the event loop prioritizes enqueueing another
render or handling user input over executing runnables.

It's a subtle change as a result of a week of digging into performance
on X11. It's also not perfect: ideally we'd get rid of the intermediate
channel here and had more control over when and how we run runnables vs.
X11 events, but I think short of rewriting how we use an event loop,
this is good cost/benefit change.

To illustrate:

Before this change, it was possible to block the app from rendering for
a long time by just creating a ton of futures that were executed on the
"main" thread (we don't have a "main" thread on Linux, but we have a
single thread in which we run the event loop).

That was relatively easy to reproduce by opening the `zed` repository
and starting `rust-analyzer`: at some point `rust-analyzer` sends us so
many notifications, that are all handled in futures, that the event loop
is busy just working off the runnables, never getting to the events that
X11 sends us or our own timer to re-enqueue another render.

When you put print statements into the code to show when which event was
handled, you'd see something like this **before this change**:

```
[ ... hundreds of runnable.run() ... ]
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
new render tick timer. lag: 56.942049ms
X11 event
new render tick timer. lag: 9.668µs
X11 event
new render tick timer. lag: 9.955µs
X11 event
runnable.run()
runnable.run()
runnable.run()
runnable.run()
new render tick timer. lag: 12.462µs
X11 event
new render tick timer. lag: 14.868µs
X11 event
new render tick timer. lag: 11.234µs
X11 event
new render tick timer. lag: 11.681µs
X11 event
new render tick timer. lag: 13.926µs
X11 event
```

Note the `lag: 56ms`: that's the difference between when we wanted to
execute the callback that enqueues another render and when it ran.

Longer lags are possible, this is just the first one I grabbed from the
logs.

Now, compare this with the logs **after this change**:

```
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
new render tick timer. lag: 36.051µs
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
X11 event
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
runnable.run()
```

In-between many `runnable.run()` we'll always handle events.

So, in essence, what this change does is to introduce 2 priorities into
the X11 event queue:

- high: X11 events (user events, render events, ...), render tick, XIM
events, ...
- low: all async rust code

I've tested this with a debug build and release build and I think the
app now feels more responsive. It doesn't feel perfect still, especially
in the slow debug builds, but I couldn't observe 10s lockups anymore.

Since it's a pretty small change, I think we should go for it and see
how it behaves.

Thanks to @maan2003 this now also includes the same change to Wayland.

Release Notes:

- N/A

---------

Co-authored-by: maan2003 <manmeetmann2003@gmail.com>

linux/x11: handle XIM events sync to reduce lag (zed-industries#12840)

This helps with the problem of keyboard input feeling laggy when the
event loop is under load.

What would previously happen is:

- N events from X11 arrive
- N events get forwarded to XIM
- N events are handled in N iterations of the event loop (sadly, yes: we
only seem to be getting back one `ClientMessage` per poll from XCB
connection)
- Each event is pushed into the channel
- N event loop iterations are needed to get the events off the channel
and handle them

With this change, we get rid of the last 2 steps: instead of pushing the
event onto a channel, we store it on the XIM handler itself, and then
work it off synchronously.

Usually one shouldn't block the event loop, but I think in this case -
user input! - it's better to handle the events directly instead of
re-enqueuing them again in a channel, where they can accumulate and need
multiple iterations of the loop to be worked off.

This does *not* fix the problem of input feeling choppy/slower when the
system is under load, but it makes the behavior now feel exactly the
same as when XIM is disabled.

I also think the code is easier to understand since it's more
straightforward.

Release Notes:

- N/A

ruby: Remove outline for running tests (zed-industries#12642)

Hi, this pull request superseeds the
zed-industries#12624
and removes queries for runnables from `outline.scm`. This pull request
has couple things to mention:

- Removed task for running tests with `minitest` as I think it's not
reliable in its state because, AFAIK, the only way to run `minitest`
with the specific line, i.e. `bundle exec rake test
spec/models/some_model.rb:12` is to use it with Rails. The support for
`minitest` is still there and users can add their own task, for
instance, when they use `minitest` in Rails to get support for running
tests:

  ```json
  {
    "label": "test $ZED_RELATIVE_FILE:$ZED_ROW",
    "command": "./bin/rails",
    "args": ["test", "\"$ZED_RELATIVE_FILE:$ZED_ROW\""],
    "tags": ["minitest-test"]
  }
  ```

**Question:** Perhaps that should be mentioned in the Ruby extension
documentation?

- Adjusted runnables queries to work without `ZED_SYMBOL`.

Release Notes:

- N/A

assistant: Add `/now` slash command (zed-industries#12856)

This PR adds a `/now` command to the Assistant for indicating the
current date and time to the model.

Release Notes:

- Added `/now` command to the Assistant for getting the current date and
time.

Show extension download counts with thousands separators (zed-industries#12857)

This PR adjusts the extension download counts to be displayed using
thousands separators.

Release Notes:

- Adjusted extension download counts to display with thousands
separators (e.g., `1,000,000`).

Extract a `proto` crate out of `rpc` (zed-industries#12852)

Release Notes:

- N/A

---------

Co-authored-by: Nathan <nathan@zed.dev>

Update linux.md

Update linux.md

Fix target of proto diff on CI (zed-industries#12861)

Release Notes:

- N/A

Add missing LICENSE file to `proto` crate (zed-industries#12863)

This PR adds a missing LICENSE file to the recently-extracted `proto`
crate.

Release Notes:

- N/A

Remove unused `color` crate (zed-industries#12860)

This PR removes the `color` crate, as it was not used anywhere.

We had added this experimentally, but right now its existence is just a
source of confusion.

Release Notes:

- N/A
ming900518 pushed a commit to ming900518/zed that referenced this issue Jun 14, 2024
Co-Authored-By: ethanmsl@gmail.com

Release Notes:

- vim: Added `gu`/`gU`/`g~` for changing case. (zed-industries#12565)
@github-actions github-actions bot added admin read Pending admin review triage Maintainer needs to classify the issue labels Nov 5, 2024
@notpeter notpeter removed triage Maintainer needs to classify the issue admin read Pending admin review labels Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [core label] vim
Projects
None yet
Development

No branches or pull requests

3 participants