luckasRanarison / nvim-devdocs

Neovim DevDocs integration
MIT License
270 stars 20 forks source link

sections: only display after section start #42

Closed emmanueltouzery closed 1 year ago

emmanueltouzery commented 1 year ago

fixes #28 It was wrong to give all the contents to the preview/display command and then search on the output, because the command could change the format completely. So now we submit to the converter program only data after the section start, and we don't need to search the output anymore.

emmanueltouzery commented 1 year ago

ideally we'd also drop the content after the next section start. That would make the output shorter, and consistent (only show for the current section), and it would also speed up display for glow massively. But I went for the quickfix for now.

luckasRanarison commented 1 year ago

This is a really interesting idea, making processing before rendering was a nice thought. But there's an issue with it, docs sections are not necessarily headers, it can also be a link, a word in the inside another section, etc... and in that case the docs would get cut without having the possibility to scroll up. It's a nice workaround for the case where the section is an header though, so we should add a check and use search as a fallback.

emmanueltouzery commented 1 year ago

interesting... but how would I know whether it's a header or not?

I printed the contents of selection in open_doc, and I get:


{
  display = "[lodash-4] _.keys",
  index = 1491,
  ordinal = "[lodash-4] _.keys",
  value = {
    alias = "lodash-4",
    link = "index#keys",
    name = "[lodash-4] _.keys",
    path = "1,### _.keys(object)[source](https://github.com/lodash/lodash/blob/4.17.10/lodash.js#L13305) [npm package](https://www.npmjs.com/package/lodash.keys)"
  }
}

I can't say here anything that tells me whether it's a header or not.

I have played a little also with converting the header text with glow to search for the converted header text. It would possibly be workable, but it's very messy, because glow emits shell escape codes for coloring. These would have to be stripped for searching. I in fact have some code to do that for some other project, but it just seems very frail...

emmanueltouzery commented 1 year ago

i think i have a better idea, hold on...

emmanueltouzery commented 1 year ago

I now opened an alternative PR, #45

Keeping this one open in case you still prefer the approach from this PR, after adaptations.

luckasRanarison commented 1 year ago

interesting... but how would I know whether it's a header or not?


{
  ...
  value = {
    ...
    path = "1,### _.keys(object)[source](https://github.com/lodash/lodash/blob/4.17.10/lodash.js#L13305) [npm package](https://www.npmjs.com/package/lodash.keys)"
  }
}

By header I meant markdown headers, starting with #. I think I prefer this PR approach, it could also be extended to omit next sections. I honestly prefer making preprocessing like this instead of messing more with the transpiler. I gave up the idea of splitting the docs further because of the previous facts: sections not necessarily being headers and the nested sections. I don't want to add more complexity there just to handle some edge cases. I'll work on this tomorrow...

emmanueltouzery commented 1 year ago

I understand. None of the two PRs touch the transpiler, yes. For now I won't do anything because you wrote that you'll work on that. If I can help, let me know! And thank you for the plugin!

luckasRanarison commented 1 year ago

I'm also really thankful for all your help, I've learned so much with you :)

luckasRanarison commented 1 year ago

It's really promising, still a wip but the idea is there. Playing with lines is easier than playing with the nodes for searching sections, and the processing is not expensive. This is really the way to go. I'll change the previewer implementation and start filtering there, this would finally fix all major issues!!! Thanks for the idea :)

luckasRanarison commented 1 year ago

Also forget about sections not being headers, I noticed this with bash docs but it still works well and the docs are still concise enough even if the section start is left off.

luckasRanarison commented 1 year ago

The only remaining problem is piping command with the telescope previewer. Before I just used the filepath with the cmd (glow) but now we have to read the file first and pipe the filtered lines to the cmd and I don't know how.

local term_doc_previewer = previewers.new_termopen_previewer({
  title = "Preview",
  get_command = function(entry)
    local splited_path = vim.split(entry.value.path, ",")
    local file = splited_path[1]
    local file_path = path:new(plugin_config.dir_path, "docs", entry.value.alias, file .. ".md")
    local lines = file_path:readlines()
    local filtered_lines = operations.filter_doc(lines, splited_path[2])
    local args = {} -- TODO

    return args
  end,
})
emmanueltouzery commented 1 year ago

about the termopen issue, I'm optimistic. See https://github.com/nvim-telescope/telescope.nvim/blob/master/lua/telescope/previewers/term_previewer.lua#L217

Telescope has a lot of options that you can see only in the code, not in the docs. Here we can see the send_input option. I think you can use it to send data to glow via stdin instead of giving a file name.

emmanueltouzery commented 1 year ago

more about the telescope previewer options, including send_input: https://github.com/nvim-telescope/telescope.nvim/blob/master/lua/telescope/previewers/init.lua#L63

emmanueltouzery commented 1 year ago

some example of use of send_input: https://github.com/gallavee/search_results.nvim/blob/c8b2d9824a204227f429d9b1eac7d40b40d1ae75/plugin/main.lua#L147

emmanueltouzery commented 1 year ago

i think you'd call self.send_input in the preview callback.

luckasRanarison commented 1 year ago

i think you'd call self.send_input in the preview callback

It'd be too late, the string should be passed before the command, the simplest way to do this would be echo "# Hello world" | glow -p but we can't pipe commands. I tried another approach without using termopen previewer but a normal buffer previewer and then output a job result like the in the normal preview buffer, but the lines get wrapped and the output is really messy.

emmanueltouzery commented 1 year ago

I played with it a little, but didn't get very far for now. Maybe we should ask the telescope developers, I think they have a chat.

The approach I suggested yesterday is more the telescope low-level API, not higher-level like termopen or buffer previewers.

Piping would be possible also with termopen but it gets very ugly: https://github.com/debugloop/telescope-undo.nvim/blob/3dec002ea3e7952071d26fbb5d01e2038a58a554/lua/telescope-undo/previewer.lua#L27

Another option possibly could be to create a temporary file, pass it to glow, overwrite it every time, then deleting it in the teardown callback maybe? Although teardown is available to the buffer previewer but not to the termopen previewer.

Maybe you can make a branch with the buffer previewer attempt. Possibly we could check what does the termopen previewer does differently from the buffer previewer, maybe it's possible to fix the wrapping...

emmanueltouzery commented 1 year ago

Maybe you can make a branch with the buffer previewer attempt

i meant so that maybe i can try to play with it, if you put it in a branch. Although I can't guarantee i'll have time to work on it very soon.

emmanueltouzery commented 1 year ago

Otherwise if this gets really messy I'd possibly reconsider generating "cut files" at fetching time, after the transpiling. It would be a second pass, not touching the existing transpiling code.

luckasRanarison commented 1 year ago

Maybe you can make a branch with the buffer previewer attempt. Possibly we could check what does the termopen previewer does differently from the buffer previewer, maybe it's possible to fix the wrapping...

https://github.com/luckasRanarison/nvim-devdocs/tree/telescope-previewer In this version I sent the job result to the picker preview buffer instead of using termopen_preview, the same method used for rendering the docs before. And so the pager -p arg is no longer needed. I've never realized it but even before the lines got wrapped but with this version there are extra lines for some reason.

luckasRanarison commented 1 year ago

Otherwise if this gets really messy I'd possibly reconsider generating "cut files" at fetching time, after the transpiling. It would be a second pass, not touching the existing transpiling code.

I really want to avoid this one. Because of nested sections, we'll need to process a single file multiple times, the building process is already slow enough. And I don't even want to imagine the number of resulting files...

luckasRanarison commented 1 year ago

Oh wait, I'm dumb. It actually works the extra lines were added because I used the same args for the normal previewer and picker previewer and the -w was set to 97 which caused extra lines!

luckasRanarison commented 1 year ago

We've finally did it!!!

emmanueltouzery commented 1 year ago

:tada: :rocket: :dancers:

emmanueltouzery commented 1 year ago

fantastic, looking forward to testing this tonight and i'm super optimistic that this is it :tada:

luckasRanarison commented 1 year ago

🚀 👯 🎉 :) I forgot to specify the change with the picker cmd args in the commit description lol.

emmanueltouzery commented 1 year ago

I checked quickly. it was so fast that at first I thought there's a bug and it doesn't go through glow :)

Great work, thanks!