Skip to content

Conversation

@fwastring
Copy link
Contributor

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

@fwastring
Copy link
Contributor 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
Contributor Author

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

@fwastring
Copy link
Contributor 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
Contributor 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
Contributor 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
Contributor 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 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

@fwastring fwastring force-pushed the main branch 2 times, most recently from b54fceb to 6f4237f Compare December 9, 2025 17:45
@fwastring
Copy link
Contributor 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.

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
Contributor Author

There we go, trimmed down the settingsExample and switched the commit order :) @GaetanLepage

Copy link
Member

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

Thanks for the corrections @fwastring!

@GaetanLepage GaetanLepage added this pull request to the merge queue Dec 11, 2025
Merged via the queue into nix-community:main with commit 1665448 Dec 11, 2025
4 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.

4 participants