search

dlvhdr / gh-dash

A beautiful CLI dashboard for GitHub 🚀
https://dlvhdr.github.io/gh-dash
MIT License
7.06k stars 208 forks source link

Support `:owner/:repo` template in repoPaths configuration #343

Closed rmacklin closed 7 months ago

rmacklin commented 7 months ago

Summary

Previously, we supported a "wildcard" mapping like

user1/*: /path/to/user1_repos/*

to map any repository owned by user1 to the directory of the same name within /path/to/user1_repos/.

But for folks who store all GitHub repositories under a path that includes the owner's name and the repo's name, this would still require them to enter a wildcard entry for every owner. To avoid that, this PR introduces support for an :owner/:repo template in the repoPaths configuration. Now we can specify a repoPaths entry like:

:owner/:repo: /path/to/github.com/:owner/:repo

which will cover everything. It can still be overridden with individual entries that specify either an exact repository or a specific owner wildcard.

How did you test this change?

I added more test cases to ui/common/repopath_test.go. I also built it locally and ran ./gh-dash using an :owner/:repo template in my repoPaths configuration and confirmed that it was now able to successfully check out a PR using my configured location.

rmacklin commented 7 months ago

Thank you for reviewing and merging! And for creating this cool TUI :)

BTW, one thing that I'm thinking about is that we don't really need the support for default_path anymore - e.g. this configuration:

default_path: ~/code/repos

can be achieved using the more general template support introduced in this PR via:

:owner/:repo: ~/code/repos/:repo

(In contrast, the example in the PR description cannot be achieved using default_path)

Perhaps we could deprecate default_path and remove it in the next major release.

rmacklin commented 7 months ago

Another option would be to extend default_path with the template functionality, but personally I prefer keeping the :owner/:repo key since it 1) makes the value a bit more self-explanatory and 2) aligns better with the A/B format of the other supported keys:

some-owner/some_repo: /path/to/some_repo
another-owner/*: /path/to/another-owner/*
:owner/:repo: /path/to/:owner/:repo

Additionally, even changing the behavior of default_path to support substitutions could technically be a breaking change (for some users), so if we want to be strict about that we'd probably want to do so in a new major version anyway.

I've opened a PR here for deprecating default_path: https://github.com/dlvhdr/gh-dash/pull/344 - let me know what you think.

dlvhdr commented 7 months ago

Yeah totally agreed. We need to consolidate the way to define these paths and choose one method. I like your suggestion. :path vars like in urls work well here. I'm open to introducing a breaking change.

rmacklin commented 7 months ago

So would you like to go straight to the breaking change and skip the soft deprecation PR? If so, I can close #344 and remove default_path