Repository navigation
feat(info): show driver docs URL in dbc info - #511
djfrancesco wants to merge 2 commits into
Conversation
Display the registry's docs_url for a driver in `dbc info` output: a "Docs:" line in plaintext (only when set) and a docs_url field in the --json payload (always present, empty when unset). Closes columnar-tech#499
amoeba
left a comment
There was a problem hiding this comment.
This looks great. Just two small changes.
Thanks for taking the time to contribute here!
| b.WriteString(bold.Render("License: ") + drv.License + "\n") | ||
| b.WriteString(bold.Render("Description: ") + drv.Desc + "\n") | ||
| if drv.DocsURL != "" { | ||
| b.WriteString(bold.Render("Docs: ") + drv.DocsURL + "\n") |
There was a problem hiding this comment.
What do you think about using Hyperlink here? Many modern/popular terminals have auto-detection and don't need OSC8 control codes to make hyperlinks clickable but for (1) users who turn auto-detection off or (2) use terminals that don't support OSC8 control codes, this is slightly better. It looks like I missed doing this to the URL in dbc docs so we could do a follow-up PR to fix that.
I also think we should use the full word "Documentation" here. I take your point about using "Docs" to which matches the subcommand but the other fields here don't abbreviate.
| b.WriteString(bold.Render("Docs: ") + drv.DocsURL + "\n") | |
| b.WriteString(bold.Render("Documentation: ") + lipgloss.NewStyle().Hyperlink(drv.DocsURL).Underline(true).Render(drv.DocsURL) + "\n") |
There was a problem hiding this comment.
Thank you so much for your time @amoeba !
I agree with the full word "Documentation" instead of "Docs". No need for shortening...
About OSC 8, I didn't know about it but I like everything that can be done in the terminal.
Both changes are in b6a1ca7: Documentation: as the label, and the URL rendered with Hyperlink + underline. I had to add the charm.land/lipgloss/v2 import to info.go for it to build. Piped output is still plain text, since lipgloss.Println strips the escape codes.
I'd be happy to do the dbc docs follow-up too.
| "Available Packages:\n"+ | ||
| " - linux_amd64\n - macos_amd64\n"+ | ||
| " - macos_arm64\n - windows_amd64", out) | ||
| suite.NotContains(out, "Docs:") |
There was a problem hiding this comment.
| suite.NotContains(out, "Docs:") | |
| suite.NotContains(out, "Documentation:") |
Address review feedback: spell out the "Documentation:" label to match the other unabbreviated fields, and render the URL as an underlined OSC 8 hyperlink so it is clickable in terminals without URL auto-detection. Escape codes are still stripped when stdout is not a terminal.
Closes #499
Summary
dbc docsalready uses thedocs_urlfrom the registry index, butdbc infonever showed it. This PR adds it to both the plaintext and the JSON output.In plaintext, there's a new
Docs:line afterDescription:. It only shows up when the driver has a docs URL:With
--json, thedriver.infopayload gets adocs_urlfield:{"schema_version":1,"kind":"driver.info","payload":{"driver":"snowflake", ..., "description":"...","docs_url":"https://adbc-drivers.org/drivers/snowflake/","packages":[...]}}A few choices I made
I went with
Docs:as the label since it's short and matchesdbc docs. Drivers without a docs URL get no line at all, so their output doesn't change.In the JSON,
docs_urlis always there (empty string when unset). I left outomitemptyon purpose, so it behaves liketitle,licenseanddescriptionright next to it. The field is new, soschema_versionstays at 1.Easy to change any of these if you'd prefer something else.
The tests reuse the existing
test-driver-docs-urlfixture, and also check that a driver without a docs URL gets noDocs:line and an emptydocs_url.Not in this PR
dbc search -v --jsondoesn't includedocs_urleither. I can do that in a follow-up if you want it.Test plan
go test ./...passes with Go 1.26.8 (env and user levels)go vet ./...clean; changed files aregofmt-cleandbc info snowflakeanddbc info snowflake --jsonagainst the default registry show the docs URL