Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

VS.Vim.Window.FloatingWindow: Support on_opened/on_closed as call… #3

Merged
merged 31 commits into from
Jan 17, 2021
Merged

VS.Vim.Window.FloatingWindow: Support on_opened/on_closed as call… #3

merged 31 commits into from
Jan 17, 2021

Conversation

hrsh7th
Copy link
Owner

@hrsh7th hrsh7th commented Jan 10, 2021 •
edited
Loading

…back

TODO

  • Add more tests
  • Check API design

@hrsh7th hrsh7th force-pushed the enhance-floating-window branch 4 times, most recently from 01375e3 to c57cc2b Compare January 10, 2021 07:28
@hrsh7th
Copy link
Owner Author

hrsh7th commented Jan 10, 2021

The widget api is experimental. I will want to change the API later...

hrsh7th
hrsh7th commented Jan 10, 2021
View reviewed changes
autoload/vital/__vital__/VS/Vim/Window/Widget/MarkupContent.vimspec Outdated
\ '```',
\ ])
call l:markup_content.close()
End
Copy link
Owner Author

Choose a reason for hiding this comment

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

Hm... the border is complex to calculate position and size...

Copy link

Choose a reason for hiding this comment

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

there are multiple implementations for border.

https://github.com/nvim-lua/popup.nvim
https://github.com/lambdalisue/loupe.vim/blob/master/autoload/loupe/viewer/nvim.vim#L24-L105

neovim in vim-lsp doesn't support border so might be we can implement borders later? That way I can at least try converting in vim-lsp neovim to use this floatingwindow.

@hrsh7th hrsh7th force-pushed the enhance-floating-window branch from 5f64035 to 49e6e12 Compare January 12, 2021 16:26
prabirshrestha
prabirshrestha reviewed Jan 13, 2021
View reviewed changes
autoload/vital/__vital__/VS/Vim/Window.vim Outdated
"
" scroll
"
" NOTE: We can't test it because it uses timer.
Copy link

Choose a reason for hiding this comment

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

curious any reason we use timer?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Because it will use with mapping.

inoremap <expr><C-f> vital#vital#import('VS.Vim.Window').scroll(+4)
inoremap <expr><C-u> vital#vital#import('VS.Vim.Window').scroll(-4)

Copy link
Owner Author

Choose a reason for hiding this comment

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

You review is insightful to me. I've change the design.

autoload/vital/__vital__/VS/Vim/Window.vim Outdated
if s:scrollable(l:win)
let l:ctx = {}
function! l:ctx.callback(win, delta) abort
let l:height = line('w$') - line('w0')
Copy link

Choose a reason for hiding this comment

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

since this is a callback due to timer, the window could be deleted. should there be some guard on win exist?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Agree. I will add it.

autoload/vital/__vital__/VS/Vim/Window/Widget/MarkupContent.vim Outdated
" @param {string[]} contents
"
function! s:MarkupContent.open(row, col, contents) abort
call deletebufline(self.bufnr, '^', '$')
Copy link

Choose a reason for hiding this comment

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

not required now but neovim has an api to do bulk edits. not sure if it is already in master or in pr.

@hrsh7th
Copy link
Owner Author

hrsh7th commented Jan 13, 2021 

nnoremap <expr> <C-f> <SID>scroll(+4)
nnoremap <expr> <C-u> <SID>scroll(-4)
inoremap <expr> <C-f> <SID>scroll(+4)
inoremap <expr> <C-u> <SID>scroll(-4)

function! s:scroll(count) abort
  let l:Window = vital#vital#import('VS.Vim.Window')
  let l:win = l:Window.find({ win ->
  \   l:Window.get_var(win, 'scrollable')
  \ })
  call timer_start(0, { -> l:Window.scroll(l:win, l:Window.info(l:win).topline + a:count) })
  return "\<Ignore>"
endfunction

@hrsh7th hrsh7th force-pushed the enhance-floating-window branch from cd59582 to 4aef838 Compare January 13, 2021 08:07
@hrsh7th
Copy link
Owner Author

hrsh7th commented Jan 14, 2021

I decided to remove VS.Vim.Window.Widget.MarkupContent for now.

The Widget API is very difficult to design.

@hrsh7th hrsh7th force-pushed the enhance-floating-window branch from 0d1769e to b956d45 Compare January 16, 2021 12:37
@prabirshrestha prabirshrestha mentioned this pull request Jan 16, 2021
Closed
@hrsh7th hrsh7th force-pushed the enhance-floating-window branch from 67120a2 to 014fda7 Compare January 17, 2021 04:41
@hrsh7th
Copy link
Owner Author

hrsh7th commented Jan 17, 2021

In this branch, The window can be scrolled by the below codes.

nnoremap <expr> <C-f> <SID>scroll(+4)
nnoremap <expr> <C-u> <SID>scroll(-4)
inoremap <expr> <C-f> <SID>scroll(+4)
inoremap <expr> <C-u> <SID>scroll(-4)

function! s:scroll(count) abort
  let l:ctx = {}
  function! l:ctx.callback() abort closure
    let l:Window = vital#vital#import('VS.Vim.Window')
    for l:winid in l:Window.find({ winid -> getwinvar(winid, 'lsp_scrollable') == 1 })
      let l:info = l:Window.info(l:winid)
      call l:Window.scroll(l:winid, l:info.topline + a:count)
    endfor
  endfunction
  call timer_start(0, { -> l:ctx.callback() })
  return "\<Ignore>"
endfunction

@hrsh7th
Copy link
Owner Author

hrsh7th commented Jan 17, 2021

Currently, the border is not supported but It can be merged now, I think.

@hrsh7th hrsh7th merged commit d16a7aa into master Jan 17, 2021
@hrsh7th hrsh7th mentioned this pull request Jan 17, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prabirshrestha prabirshrestha prabirshrestha left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hrsh7th @prabirshrestha