Skip to content

plugins/hunk: init #3293

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jalil-salame
Copy link

Add support for hunk.nvim, a plugin for splitting diffs.

Also add myself as a maintainer for it.

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! Here is some initial feedback.

Comment on lines 14 to 18
extraOptions = {
fileTypeIcons = lib.mkEnableOption ''
Enable file type icons in the file tree.
'';
};
Copy link
Member

Choose a reason for hiding this comment

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

I have missed something or this option is only used in the warning?

Copy link
Author

Choose a reason for hiding this comment

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

Nope, only in the warning, I tried setting web-devicons to true there, but it threw a warning.

@jalil-salame jalil-salame force-pushed the push-yxsyxkonwotv branch 2 times, most recently from 3de2637 to 714e66e Compare May 7, 2025 07:29
@jalil-salame jalil-salame requested a review from GaetanLepage May 7, 2025 07:30
Comment on lines 14 to 18
extraOptions = {
fileTypeIcons = lib.mkEnableOption ''
warning if file type icons are not available.
'';
};
Copy link
Member

Choose a reason for hiding this comment

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

Having an enable option for a warning feels like an anti-pattern

Suggested change
extraOptions = {
fileTypeIcons = lib.mkEnableOption ''
warning if file type icons are not available.
'';
};

Copy link
Author

Choose a reason for hiding this comment

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

Removed the option!

Add support for [hunk.nvim][1], a plugin for splitting diffs.

Also add myself as a maintainer for it.

[1]: https://github.com/julienvincent/hunk.nvim
Comment on lines +139 to +145
jalil-salame = {
email = "[email protected]";
github = "jalil-salame";
githubId = 60845989;
name = "Jalil David Salamé Messina";
keys = [ { fingerprint = "7D6B 4D8F EBC5 7CBC 09AC 331F DA33 17E7 5BE9 485C"; } ];
};
Copy link
Member

Choose a reason for hiding this comment

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

Can you move this change into a dedicated commit, so you end up with two:

  1. maintainers: add jalil-salame
  2. plugins/hunk: init

(in that order)

Comment on lines +15 to +17
Either `plugins.web-devicons` or `plugins.mini`* must be enabled to have icons in the file tree.

*If using `plugins.mini`, you must enable the `icons` module.
Copy link
Member

Choose a reason for hiding this comment

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

Need to escape the *, as markdown uses them for italics and bullet points.

Suggested change
Either `plugins.web-devicons` or `plugins.mini`* must be enabled to have icons in the file tree.
*If using `plugins.mini`, you must enable the `icons` module.
Either `plugins.web-devicons` or `plugins.mini`\* must be enabled to have icons in the file tree.
\*If using `plugins.mini`, you must enable the `icons` module.

Also, "must" feels too strong for an optional dependency. I'd suggest something like:

If you wish to display icons in the file tree, you should enable either plugins.web-devicons or plugins.mini. If using plugins.mini, you must enable the icons module.

?

with-web-devicons = {
plugins.hunk = {
enable = true;
fileTypeIcons = true;
Copy link
Member

@MattSturgeon MattSturgeon May 11, 2025

Choose a reason for hiding this comment

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

CI failure:

error: The option `plugins.hunk.fileTypeIcons' does not exist.

I guess this was missed while resolving #3293 (comment)?

Suggested change
fileTypeIcons = true;

with-mini = {
plugins.hunk = {
enable = true;
fileTypeIcons = true;
Copy link
Member

Choose a reason for hiding this comment

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

Same CI failure

Suggested change
fileTypeIcons = true;

package = "hunk-nvim";

description = ''
A tool for splitting diffs in Neovim
Copy link
Member

Choose a reason for hiding this comment

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

Grammar nit:

Suggested change
A tool for splitting diffs in Neovim
A tool for splitting diffs in Neovim.

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.

3 participants