Skip to content

Conversation

@fwastring
Copy link

Added support for substitute.nvim.
Added tests and plugin support, and myself as a maintainer.

@fwastring
Copy link
Author

Thank you for the quick response! I have changed the code as per your suggestions :)

Copy link
Member

@HeitorAugustoLN HeitorAugustoLN left a comment

Choose a reason for hiding this comment

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

Everything else LGTM

@fwastring
Copy link
Author

Good catch, changed it so that aligns with that now.

@fwastring
Copy link
Author

Fixed the ... of the inputs, the other comment was already resolved in an older commit.

Copy link
Member

@HeitorAugustoLN HeitorAugustoLN left a comment

Choose a reason for hiding this comment

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

LGTM, I think you might need to squash your commits to:

maintainers: add fwastring
plugins/substitute: init

@fwastring
Copy link
Author

Thank you, I will do that. How do I request another review to be able to merge?

@HeitorAugustoLN
Copy link
Member

Since you are not a maintainer yet, you can't request reviews, but I will request for you

@fwastring
Copy link
Author

Could you have a look at this? @GaetanLepage

@MattSturgeon
Copy link
Member

MattSturgeon commented Dec 8, 2025

I think you might need to squash your commits to:

maintainers: add fwastring
plugins/substitute: init

This still needs to be done.

Otherwise, LGTM.

@fwastring
Copy link
Author

I think I squashed the commits now @MattSturgeon

@GaetanLepage
Copy link
Member

I think I squashed the commits now @MattSturgeon

Sorry for the misunderstanding. There needs to be two commits:

  • maintainers: add fwastring
  • plugins/substitute: init

Currently, there is only a single one.

Comment on lines +12 to +37
settingsExample = {
on_substitute = lib.nixvim.nestedLiteralLua ''
function(params)
vim.notify("substituted using register " .. params.register)
end
'';

yank_substituted_text = true;
modifiers = [
"join"
"trim"
];
highlight_substituted_text.timer = 750;
range = {
prefix = "S";
complete_word = true;
confirm = true;
subject.motion = "iw";
range.motion = "ap";
suffix = "| call histdel(':', -1)";
auto_apply = true;
cursor_position = "start";
};
exchange = {
motion = "ap";
use_esc_to_cancel = false;
preserve_cursor_position = true;
};
};
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Optionally reduce the size of settingsExample as it will make the documentation clearer.

Comment on lines 1 to 4
{
lib,
...
}:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{
lib,
...
}:
{ lib, ... }:

nit

plugins/substitute: removed options and used nested

plugins/substitute: changed example

plugins/substitute: remove __raw

plugins/substitute: Fixed failing test

plugins/substitute: removedplugins/substitute: init

removed
@fwastring
Copy link
Author

Squashed to two commits now @GaetanLepage 👍🏻
Fixed one of the nits, but didn't know how to make the settingsExample smaller so I kept it all.

@GaetanLepage
Copy link
Member

Squashed to two commits now @GaetanLepage 👍🏻 Fixed one of the nits

Commits are in the wrong order. The maintainers one should be first.

but didn't know how to make the settingsExample smaller so I kept it all.

Simply drop a few options. I don't think that they are interdependent, are they?
It's fine to keep it as is too.

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.

4 participants