Skip to content

Conversation

@phinze
Copy link
Contributor

@phinze phinze commented Nov 11, 2025

👋 Hey there! This is my first contribution to nixvim - please let me know if I missed anything or if there are any improvements I should make!


Summary

Add support for gitportal.nvim, a plugin that bridges Neovim and Git hosting platforms.

What does this plugin do?

GitPortal enables users to:

  • Open the current file in their browser with optional line ranges
  • Open shareable Git host permalinks directly in Neovim while respecting branch/commit context

Changes in this PR

  • Add plugin definition with full configuration support
  • Include all 6 available settings options:
    • always_include_current_line - Always include the current line in URLs
    • always_use_commit_hash_in_url - Use commit hash instead of branch name
    • browser_command - Custom browser command (auto-detection if null)
    • default_remote - Default git remote to use (defaults to "origin")
    • git_provider_map - Custom mapping of git providers
    • switch_branch_or_commit_upon_ingestion - Behavior when ingesting permalinks ("always", "ask_first", or "never")
  • Add test cases for both minimal and configured usage
  • Add myself (phinze) to the maintainers list

Example Configuration

{
  plugins.gitportal = {
    enable = true;
    settings = {
      always_include_current_line = true;
      default_remote = "upstream";
      switch_branch_or_commit_upon_ingestion = "ask_first";
    };
  };
}

Testing

  • Syntax validation passed
  • Code formatted with `nix fmt`
  • Plugin follows nixvim conventions (using `mkNeovimPlugin`)
  • Test files created following the standard pattern

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.

Hi there! Welcome on board!
This PR is on a very good way.
A few initial remarks:

  • You don't have to declare the settingsOptions. We tend to move away from this as settings is already a freeform option. This means that it accepts arbitrary values and automatically converts them to lua. This reduces future maintenance.
  • Please split your patch into two commits:
    • maintainers: add phinze
    • plugins/gitportal: init

@phinze
Copy link
Contributor Author

phinze commented Nov 11, 2025

Hi @GaetanLepage thanks for the quick review! Based on the build failures it looks like I misunderstood and jumped the gun - this needs to be submitted to nixpkgs first. Will convert to draft until I can sort that out. 👍🏻

@phinze phinze marked this pull request as draft November 11, 2025 23:17
@GaetanLepage
Copy link
Member

Hi @GaetanLepage thanks for the quick review! Based on the build failures it looks like I misunderstood and jumped the gun - this needs to be submitted to nixpkgs first. Will convert to draft until I can sort that out. 👍🏻

Yes, if the plugin is not packaged there, you first need to open a PR to add it.

@phinze
Copy link
Contributor Author

phinze commented Nov 12, 2025

Ok! Upstream has merged and I believe I've addressed the feedback. The build failures should go green on the next scheduled flake update.

@phinze phinze marked this pull request as ready for review November 12, 2025 16:56
@phinze phinze changed the base branch from main to update/main November 13, 2025 16:31
@phinze phinze force-pushed the add-gitportal-plugin branch from 83c49d9 to 7079a87 Compare November 13, 2025 16:45
@phinze
Copy link
Contributor Author

phinze commented Nov 13, 2025

Rebased over the current update branch from #3934 and fixed an issue and now CI is looking good - once that lands the base branch should automatically switch to main 👍🏻

@GaetanLepage GaetanLepage deleted the branch nix-community:update/main November 14, 2025 00:10
@GaetanLepage
Copy link
Member

Rebased over the current update branch from #3934 and fixed an issue and now CI is looking good - once that lands the base branch should automatically switch to main 👍🏻

Apparently it didn't work ^^ Please, re-submit a PR.
In such a situation, it's better to simply wait for the update to be merged to main.

@phinze phinze mentioned this pull request Nov 14, 2025
@MattSturgeon
Copy link
Member

MattSturgeon commented Nov 16, 2025

once that lands the base branch should automatically switch to main 👍🏻

I wonder if rebase-merge and/or merge-queues messed that up? You're right that this PR targeting another PR's branch should cause this PR's target to switch when the other is merged:

Screenshot From 2025-05-23 20-26-52

EDIT: I suspect the update PR getting force-pushed messed things up. At the time the update PR was merged, this PR was technically no longer based on it, even though it was targeting it.

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