Skip to content

Conversation

@MariaSolOs
Copy link
Contributor

Refer to #1646

I'm very sorry for the trouble, for some reason GitHub stopped showing all my commits (??).

fix: Update docs view on entry change
@hrsh7th
Copy link
Owner

hrsh7th commented Jul 10, 2023

The implementation is good but I think the API should be cmp.visible_docs(), cmp.open_docs() and cmp.close_docs().

What do you think?

completeopt = 'menu,menuone,noselect',
keyword_pattern = [[\%(-\?\d\+\%(\.\d\+\)\?\|\h\w*\%(-\w*\)*\)]],
keyword_length = 1,
docs_initially_visible = true,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... I'll consider the better place for this option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I was unsure about it too. IMO it doesn't make sense to put it in WindowConfig since those only involve UI settings, and all other categories don't feel right either...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also just get rid of this option and let users configure if docs should be initially visible or not.

@MariaSolOs
Copy link
Contributor Author

The implementation is good but I think the API should be cmp.visible_docs(), cmp.open_docs() and cmp.close_docs().

What do you think?

Sounds reasonable to me!

@MariaSolOs
Copy link
Contributor Author

@hrsh7th lmk if this looks good to you. I can add a couple of examples and update the documentation to explain how these work :)

lua/cmp/view.lua Outdated
return
end
self.docs_view:open(e, self:_get_entries_view():info())
if self.docs_view:visible() then
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. I didn't understand why this was needed. Could you explain it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is needed in order to correctly update the documentation window when the selected entry changes.

  • If the documentation window is currently closed, then it won't be opened when the entry changes.
  • If it's open, then we ensure that its content reflects the new entry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm the issue with this is that the docs will initially be hidden. And so I think we do need the docs_initially_visible, or do you have a better idea of how to address this issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hrsh7th what are your thoughts on this :) ?

@hrsh7th
Copy link
Owner

hrsh7th commented Jul 11, 2023

Almost looks good.

@hrsh7th
Copy link
Owner

hrsh7th commented Jul 18, 2023

sorry for the delay. I've been busy with work...😭

I'll push a new commit for this PR. Please wait.

@MariaSolOs
Copy link
Contributor Author

sorry for the delay. I've been busy with work...😭

I'll push a new commit for this PR. Please wait.

No worries! Take your time, I’m just trying to help :)

@MariaSolOs
Copy link
Contributor Author

Any update on this? :)

@hrsh7th
Copy link
Owner

hrsh7th commented Aug 11, 2023

I pushed a fix commit. Can you please confirm?

@MariaSolOs
Copy link
Contributor Author

I pushed a fix commit. Can you please confirm?

I just tried it and it works great :) Thank you so much for taking the time to improve and document the feature!

@MariaSolOs MariaSolOs changed the title Add toggle_doc functionality feat: add toggle_doc functionality Aug 11, 2023
@hrsh7th hrsh7th merged commit 1c03ebc into hrsh7th:main Aug 12, 2023
@MariaSolOs MariaSolOs deleted the toggle-doc branch August 12, 2023 03:19
williamboman added a commit to williamboman/nvim-cmp that referenced this pull request Oct 26, 2023
…indow

* upstream/main:
  fix: remove usages from vim.lsp.util.parse_snippet (hrsh7th#1734)
  ci: fix broken tests (hrsh7th#1729)
  Format with stylua
  docs: Add comments and type annotations for sorting comparators (hrsh7th#1687)
  fix(ghost_text): `ephemeral` causes a false positive of no inline support (hrsh7th#1645)
  perf(core): simplify and improve `find_line_suffix()` (hrsh7th#1675)
  Format with stylua
  feat: add toggle_doc functionality (hrsh7th#1647)
  add context check for invalid detection
  implement is_invalid detection
  perf: fix `nvim_replace_termcodes` being called on every `CursorMoved` (hrsh7th#1650)
  improve pattern handling
  Format with stylua
  fix confirm resolve
@bohanubis
Copy link

so how to acutally use this like what is the function that I can bind it into a binding

@astier
Copy link

astier commented Jun 14, 2024

local cmp = require('cmp')
cmp.setup({
  view = { docs = { auto_open = false } },
  mapping = cmp.mapping.preset.insert({
    ['K'] = cmp.mapping(function(fallback)
      if cmp.visible_docs() then
        cmp.close_docs()
      elseif cmp.visible() then
        cmp.open_docs()
      else
        fallback()
      end
    end),
  })
})

@MariaSolOs
Copy link
Contributor Author

@beh-10257 read the docs:

additionally, if you want to open/close docs view via your key mapping, you

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.

4 participants