Skip to content

Conversation

FredeHoey
Copy link
Contributor

@FredeHoey FredeHoey commented Aug 26, 2025

Adds https://github.com/t-troebst/perfanno.nvim

perfanno-nvim was just recently added to nixpkgs, so cannot be merged until flake.lock is updated

see NixOS/nixpkgs#436798

@FredeHoey FredeHoey changed the title Draft: plugins/perfanno: add perfanno-nvim plugin plugins/perfanno: add perfanno-nvim plugin Aug 30, 2025
@FredeHoey FredeHoey force-pushed the plugins/perfanno/init branch 2 times, most recently from f1a79c0 to 7355b55 Compare August 30, 2025 16:49
@MattSturgeon

This comment was marked as resolved.

@FredeHoey

This comment was marked as resolved.

@FredeHoey FredeHoey force-pushed the plugins/perfanno/init branch from 7355b55 to 29544e5 Compare August 30, 2025 20:21
@MattSturgeon

This comment was marked as resolved.

@FredeHoey

This comment was marked as resolved.

@MattSturgeon

This comment was marked as resolved.

@GaetanLepage
Copy link
Member

I rebased on main.

Comment on lines 8 to 9
# TODO: Remove after https://github.com/NixOS/nixpkgs/pull/438707
url = "https://github.com/t-troebst/perfanno.nvim";
Copy link
Member

Choose a reason for hiding this comment

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

NixOS/nixpkgs#438707 is in all unstable channels now, and should be part of the revision we've updated to.

It should be safe to remove the explicit URL now:

Suggested change
# TODO: Remove after https://github.com/NixOS/nixpkgs/pull/438707
url = "https://github.com/t-troebst/perfanno.nvim";

Note to self: should we throw when url is explicitly defined the same as the package's meta.homepage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that an assertion would be great. I almost feel like a build should assert if both define a url, regardless of the value, because and then assume the upstream is always correct

@FredeHoey
Copy link
Contributor Author

@MattSturgeon I added the explicit require. Do you still think I should use literalLua for the example?

@MattSturgeon
Copy link
Member

MattSturgeon commented Sep 10, 2025

Testing locally with nix run .#docs:

(or without cloning: nix run github:nix-community/nixvim?ref=pull/3653/merge#docs)

image

Personally, I'd prefer using literalExpression or nestedLiteralLua, but this isn't a blocker.

@FredeHoey
Copy link
Contributor Author

FredeHoey commented Sep 10, 2025 via email

@FredeHoey FredeHoey force-pushed the plugins/perfanno/init branch from d8b0ce3 to cd8386c Compare September 10, 2025 18:41
maintainers = [ lib.maintainers.fredeb ];

settingsExample = {
line_highlights = lib.nixvim.literalLua ''require("perfanno.util").make_bg_highlights(nil, "#CC3300", 10)'';
Copy link
Member

@MattSturgeon MattSturgeon Sep 10, 2025

Choose a reason for hiding this comment

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

Sadly, using the wrong one doesn't cause a CI failure, but I believe we need nestedLiteralLua here instead of literalLua.

You may also wish to switch to using single quotes in your lua, as I don't think it is smart enough to render a '' string instead of ", unless the string includes newlines.

Or if you want full control, you can render the entire example yourself manually using a literalExpression.

You can confirm how it renders locally, using nix run .#docs (which may be a long build on the initial run).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair. I did kick off the docs build, but it's still going, so I'm still waiting for the result. I'm at 7650/8389 now, so I'll wait for it to finish and validate

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's a shame our docs take so long. I've put effort in the past into re-implementing the docs more efficiently, but sadly none of those efforts ever made it over the line.

@FredeHoey FredeHoey force-pushed the plugins/perfanno/init branch from cd8386c to 57210cb Compare September 10, 2025 19:32
@FredeHoey
Copy link
Contributor Author

@MattSturgeon take 92, how we doin? 😅

Copy link
Member

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your persistence and attention to detail!

@MattSturgeon MattSturgeon added this pull request to the merge queue Sep 10, 2025
Merged via the queue into nix-community:main with commit 5b0a6eb Sep 10, 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.

3 participants