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(lsp): preserve cursor position for move_item command #532

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

MunifTanjim
Copy link
Contributor

@MunifTanjim MunifTanjim commented Oct 1, 2024

Port simrat39/rust-tools.nvim#386

pub struct Allergies(u32);

macro_rules! enum_iter {
    ($name: ident { $($variant: ident),* } ) => {
        #[derive(Debug, PartialEq, Eq, Clone)]
        pub enum $name {
            $( $variant, )*
        }

        impl $name {
            fn variants() -> Vec<$name> {
                vec![$( $name::$variant, )*]
            }
        }
    };
}

enum_iter!(Allergen {
    Eggs,
    Peanuts,
    Shellfish,
    Strawberries,
    Tomatoes,
    Chocolate,
    Pollen,
    Cats
});

impl Allergies {
    pub fn new(score: u32) -> Self {
        Allergies(score)
    }

    pub fn is_allergic_to(&self, allergen: &Allergen) -> bool {
        (self.0 & 1_u32 << allergen.clone() as u32) != 0
    }

    pub fn allergies(&self) -> Vec<Allergen> {
        Allergen::variants()
            .iter()
            .filter_map(|allergen| self.is_allergic_to(allergen).then_some(allergen.clone()))
            .collect()
    }
}

Copy link
Contributor

github-actions bot commented Oct 1, 2024

Review Checklist

Does this PR follow the Contribution Guidelines? Following is a partial checklist:

Proper conventional commit scoping:

  • For example, fix(lsp): some lsp-related bugfix

  • Pull request title has the appropriate conventional commit prefix.

If applicable:

  • Tested
    • Tests have been added.
    • Tested manually (Steps to reproduce in PR description).
  • Updated documentation.

Copy link
Owner

@mrcjkb mrcjkb left a comment

Choose a reason for hiding this comment

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

Thanks 🙏

-- move it baby
local function handler(_, result, ctx)
if result == nil or #result == 0 then
return
end
local overrides = require('rustaceanvim.overrides')
overrides.snippet_text_edits_to_text_edits(result)
local cursor = extract_cursor_position(result)
Copy link
Owner

Choose a reason for hiding this comment

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

question: This function does more than just extracting the cursor position, as it also implements the snippet_text_edits_to_text_edits logic.
Can it be reduced to just the single functionality of extracting the cursor position?

Correct me if I'm wrong; I don't think we'll see any meaningful performance gains by looping over the text edits once.

On a related note: I'm wondering if we even need to apply snippet_text_edits_to_text_edits here.
It seems unlikely to me that moving an item would result in a snippet text edit. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

question: This function does more than just extracting the cursor position, as it also implements the snippet_text_edits_to_text_edits logic.
Can it be reduced to just the single functionality of extracting the cursor position?

Yes, sorry about that. I just copy-pasted my old PR. Updated now.

On a related note: I'm wondering if we even need to apply snippet_text_edits_to_text_edits here.
It seems unlikely to me that moving an item would result in a snippet text edit. What do you think?

rust-analyzer sends that snippet text edit, nothing we can do here but to patch it for vim.lsp.util.apply_text_edits.

@mrcjkb mrcjkb merged commit a07bb0d into mrcjkb:master Oct 1, 2024
5 checks passed
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.

2 participants