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

feat: code actions - document edits #478

Merged
merged 18 commits into from
Jul 24, 2021
Merged

feat: code actions - document edits #478

merged 18 commits into from
Jul 24, 2021

Conversation

gbaranski
Copy link
Contributor

@gbaranski gbaranski commented Jul 21, 2021

No description provided.

@gbaranski gbaranski mentioned this pull request Jul 21, 2021
Co-authored-by: Ivan Tham <pickfire@riseup.net>
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
@gbaranski
Copy link
Contributor Author

There are still some features that are not implemented yet, just because I couldn't find any code that uses them, so I can't really test if it works.

That's how it currently looks like

code-actions-helix.mp4

@gbaranski gbaranski marked this pull request as ready for review July 22, 2021 10:08
@kirawi
Copy link
Member

kirawi commented Jul 23, 2021

The clippy errors need to be addressed,

helix-term/src/commands.rs Outdated Show resolved Hide resolved
@kirawi
Copy link
Member

kirawi commented Jul 23, 2021

Looks great otherwise, though I'm not really familiar with this section of the codebase. Good job!

@gbaranski gbaranski changed the title feat: code actions feat: code actions - document edits Jul 23, 2021
@gbaranski gbaranski mentioned this pull request Jul 23, 2021
4 tasks
@gbaranski gbaranski requested a review from archseer July 23, 2021 12:27
@archseer
Copy link
Member

Switch out todo! for editor.set_error so we don't panic anymore and I think this is good to merge!

@gbaranski
Copy link
Contributor Author

Switch out todo! for editor.set_error so we don't panic anymore and I think this is good to merge!

Done, I've forgot to commit & push the changes I made few hours ago.

@archseer archseer merged commit 48e344a into helix-editor:master Jul 24, 2021
@archseer
Copy link
Member

Great work!

})
.collect();
let transaction = edits_to_transaction(doc.text(), &edits);
doc.apply(&transaction, view.id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undo doesn't work after applying a change:

Suggested change
doc.apply(&transaction, view.id);
doc.apply(&transaction, view.id);
doc.append_changes_to_history(view.id);

Copy link
Contributor Author

@gbaranski gbaranski Jul 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did work after pressing ESC, but with this change works even without it. I can't commit changes since the PR have been already merged. Maybe you could create a new PR with this change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ESC is bound to normal_mode command (even inside normal mode) and it has an append_changes_to_history call. I see you created a new PR 👍🏾

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.

5 participants