sunaku / dasht

💁 Search API docs offline, in terminal or browser
https://sunaku.github.io/dasht/man
827 stars 26 forks source link

[RFC] Add fzf support #38

Open Holzhaus opened 5 years ago

Holzhaus commented 5 years ago

A quick and dirty implementation to select a query result (#37).

sunaku commented 5 years ago

Thanks for the PR :+1: I'll merge it after some clean up. :sweat_smile:

Holzhaus commented 5 years ago

Thanks for the PR I'll merge it after some clean up.

Ah cool, I didn't expect it was already good enough to be merged. IMHO it's a bit confusing that dasht-query-html returns html code but dasht-query-fzf just returns a file URL, which also causes duplication in the dasht script. However, I didn't find a way to scroll to the correct anchor in w3m when piping output to it, so maybe there is no better way.

smackesey commented 4 years ago

Before realizing that there was an existing PR for this, I wrote a version myself that has some advantages and disadvantages vis-a-vis this one. My version:

But IMO, rather than including fzf support directly within dasht, it would be good, as suggested by @sunaku above, to generate intermediate results in a standard format and expose this functionality directly to the user. That would make this tool more open-ended and easy for the user to adapt to their needs (following the unix philosophy of "do one thing well"), without creating an open-ended precedent/burden of maintenance for various "adapters".

So what if:

It seems to me that outputting "neutral" query results by default would serve the typical user better, who wants to use dasht in some kind of editor integration pipeline etc. And unifying all the user facing scripts behind a single dasht executable is also cleaner.

I'm willing to implement the above changes if you're interested @sunaku.

smackesey commented 3 years ago

Pinging @sunaku on this-- I've made most of the above changes to my private fork and can submit a PR if you are interested in changing the interface in this way.

sunaku commented 3 years ago

Hi @smackesey, I like your idea of emitting TSV as an intermediate format in https://github.com/sunaku/dasht/pull/38#issuecomment-716885794. Please submit a PR for it, thanks.

smackesey commented 3 years ago

@sunaku Two questions:

sunaku commented 3 years ago

Hi @smackesey,

  1. Please include only these changes in the new TSV PR:
  • the intermediate result format is simply a tsv (\t-separated set of fields): name, docset, type, url
  • dasht-query-line is updated to output the intermediate result format
  • dasht-query-html is adjusted to remove the work now performed by dasht-query-line

Notably, please exclude these changes from the TSV PR:

  • main dasht executable emits the tsv results by default
  • main dasht executable takes a --browse option that triggers the current default behavior of browsing in w3m.
  • for consistency, scrap dasht-server and have main dasht executable take a --serve option that runs the code currently in dasht-server
  1. I previously added some more review comments to your other PR #53 but forgot to submit them. :sweat_smile: This is done correctly now, so you should see a "requested changes" status on the PR. Thanks.