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.

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

pyright in otter buffer doesn't update when otter was activated from markdown main document #154

Closed
stevenxxiu opened this issue Jul 2, 2024 · 18 comments
Labels
bug Something isn't working

Comments

@stevenxxiu
Copy link

otter doesn't run the LSP when code is updated in a molten document. My config is just the minimal config:

return {
  'jmbuhr/otter.nvim',
  dependencies = {
    'nvim-treesitter/nvim-treesitter',
  },
  opts = {},
}

An example document:

---
jupyter:
  jupytext:
    text_representation:
      extension: .md
      format_name: markdown
      format_version: '1.3'
      jupytext_version: 1.16.1
  kernelspec:
    display_name: Python 3 (ipykernel)
    language: python
    name: python3
---

```python
import os
```

This gives:

"os" is not accessed

But if I change os to anything else then the linter warning doesn't change.

@jmbuhr
Copy link
Owner

jmbuhr commented Jul 2, 2024

hm, I can reproduce that with a simple markdown document and otter.activate(). :LspInfo shows that otter-ls is correctly attached to the buffer and pyright is correctly attached to the otter buffer, but even if I go directly to the otter buffer (you can also see hidden buffers with :ls! and go there with :b<number of buffer>, pyright doesn't return anything when a request is sent for something other than the first line.
image
image

So naturally the same response comes when done in the main buffer via otter.nvim. So something is up with our python language server configuration.

Interestingly, I don't get the same issue when setting up otter via quarto with quarto filetype. I don't understand this, yet. Something makes pyright confused about the workspace.

@jmbuhr jmbuhr added the bug Something isn't working label Jul 2, 2024
@jmbuhr jmbuhr changed the title Doesn't run LSP when code is updated in a *molten* document pyright in otter buffer doesn't update when otter was activated from markdown main document Jul 2, 2024
@jmbuhr
Copy link
Owner

jmbuhr commented Jul 3, 2024

I have seen improvements on this when I add the handling of lsp notifications, not just requests: #155 but it still needs some work to be properly implemented.

@jmbuhr
Copy link
Owner

jmbuhr commented Jul 4, 2024

The solution was a different one, but it should work now. Let me know if it is also fixed for you.

@jmbuhr jmbuhr closed this as completed Jul 5, 2024
@stevenxxiu
Copy link
Author

stevenxxiu commented Jul 5, 2024

Hi sorry for the late reply. It's still not fixed for me. I spent some time today to make a minimal reproducible example:

local lazypath = vim.fn.stdpath('data') .. '/lazy/lazy.nvim'
if not vim.uv.fs_stat(lazypath) then
  vim
    .system({
      'git',
      'clone',
      '--filter=blob:none',
      'https://github.com/folke/lazy.nvim.git',
      '--branch=stable', -- latest stable release
      lazypath,
    })
    :wait()
end
vim.opt.runtimepath:prepend(lazypath)

vim.api.nvim_create_autocmd({ 'BufEnter', 'BufWinEnter' }, {
  pattern = '*.ipynb',
  callback = function() require('otter').activate({ 'python' }) end,
})

require('lazy').setup({
  {
    'neovim/nvim-lspconfig',
    config = function() require('lspconfig').pyright.setup({}) end,
  },
  {
    'GCBallesteros/jupytext.nvim',
    opts = {
      style = 'markdown',
      output_extension = 'md',
      force_ft = 'markdown',
    },
  },
  {
    'williamboman/mason.nvim',
    config = true,
  },
  {
    'jmbuhr/otter.nvim',
    opts = {
      lsp = {
        diagnostic_update_events = { 'BufWritePost', 'InsertLeave', 'TextChanged' },
      },
    },
    config = true,
  },
  {
    'nvim-treesitter/nvim-treesitter',
    build = ':TSUpdate',
    opts = {
      ensure_installed = {
        'markdown',
        'markdown_inline',
      },
      auto_install = true,
      highlight = {
        enable = true,
        additional_vim_regex_highlighting = false,
      },
    },
    config = function(_, opts) require('nvim-treesitter.configs').setup(opts) end,
  },
})

mason is there to make it easy to install Pyright in Docker.

An example file, call it say python.ipynb.

---
jupyter:
  jupytext:
    text_representation:
      extension: .md
      format_name: markdown
      format_version: '1.3'
      jupytext_version: 1.16.2
  kernelspec:
    display_name: Python 3 (ipykernel)
    language: python
    name: python3
---

```python
import os1
```

Change os1 to something else, and the error won't change.

@jmbuhr
Copy link
Owner

jmbuhr commented Jul 5, 2024

Thanks for the detailed reprex! I understand the problem now. You are initializing jupytext at the same time as otter (using the autocommand on ipynb files). However, otter doesn't operate on ipynb files, it operates on the markdown file created by jupytext from the ipynb file. But those things happen at the same time, so the language server that otter attaches to the python code chunks gets incomplete notifications about the state of the file and gets confused.

You can see that this is the case by removing the autocommand from your init.lua reprex

vim.api.nvim_create_autocmd({ 'BufEnter', 'BufWinEnter' }, {
  pattern = '*.ipynb',
  callback = function() require('otter').activate({ 'python' }) end,
})

and instead manually pressing :lua require'otter'.activate() once the buffer is open, in which case it should just work.

You could activate otter for the markdown filetype via an ftplugin (as this is set by jupytext as the filetype), but I would advise against that, because e.g. hover windows also have the markdown filetype, so otter would attach to all of those as well, which may get a little much.

My recommendation is to pick a filetype dedicated to computational notebooks such as Quarto (basically markdown) and configure otter to activate automatically on those. See https://github.com/GCBallesteros/jupytext.nvim?tab=readme-ov-file#configuration for configuring jupytext to use the Quarto filetype and then simply add the quarto-nvim plugin (https://github.com/quarto-dev/quarto-nvim). It will automatically activate otter for all quarto files.
You might find more inspiration in my own config, here is the part specific to Quarto: https://github.com/jmbuhr/quarto-nvim-kickstarter/blob/6455ad9123041f11e3294508db8f216223e2ca8a/lua/plugins/quarto.lua

If you want to stick with markdown you'll have to figure out a way of telling when jupytext is done with it's setup, maybe they have an autocommand you can use.

@jmbuhr
Copy link
Owner

jmbuhr commented Jul 5, 2024

image

@stevenxxiu
Copy link
Author

stevenxxiu commented Jul 6, 2024

Thanks for that. So this was a bug I introduced in the minimal config above. In my real config I indeed have nvim/after/ftplugin/markdown.lua with the following contents:

local path = vim.api.nvim_buf_get_name(0)
if string.match(path, '.ipynb$') then
  require('otter').activate({ 'javascript', 'python' })
  vim.cmd.runtime({ 'ftplugin/quarto.lua', bang = true })
end

I compared my config with Docker and fixed some issues including these:

  • Plugins cmp, mason, lspconfig, treesitter require lazy = false.
  • Plugins markdown-preview, quarto cannot have ft = { 'markdown' },.

Now the diagnostics indeed update when I open a Jupyter notebook in Neovim, using the terminal!

But the diagnostics still don't update when I open the Jupyter notebook in Neovim itself. For some reason it only works if I remove my quarto config.

This is the minimal config that reproduces the issue:

local lazypath = vim.fn.stdpath('data') .. '/lazy/lazy.nvim'
if not vim.uv.fs_stat(lazypath) then
  vim
    .system({
      'git',
      'clone',
      '--filter=blob:none',
      'https://github.com/folke/lazy.nvim.git',
      '--branch=stable', -- latest stable release
      lazypath,
    })
    :wait()
end
vim.opt.runtimepath:prepend(lazypath)

require('lazy').setup({
  {
    'neovim/nvim-lspconfig',
    config = function() require('lspconfig').pyright.setup({}) end,
  },
  {
    'GCBallesteros/jupytext.nvim',
    opts = {
      style = 'markdown',
      output_extension = 'md',
      force_ft = 'markdown',
    },
  },
  {
    'williamboman/mason.nvim',
    config = true,
  },
  {
    'jmbuhr/otter.nvim',
    opts = {
      lsp = {
        diagnostic_update_events = { 'BufWritePost', 'InsertLeave', 'TextChanged' },
      },
    },
    config = true,
  },
  {
    'quarto-dev/quarto-nvim',
    opts = {
      lspFeatures = {
        languages = { 'python' },
        chunks = 'all',
        completion = {
          enabled = true,
        },
      },
    },
  },
  {
    'nvim-treesitter/nvim-treesitter',
    build = ':TSUpdate',
    opts = {
      ensure_installed = {
        'markdown',
        'markdown_inline',
      },
      auto_install = true,
      highlight = {
        enable = true,
        additional_vim_regex_highlighting = false,
      },
    },
    config = function(_, opts) require('nvim-treesitter.configs').setup(opts) end,
  },
})

How should I be setting up quarto properly?

@jmbuhr
Copy link
Owner

jmbuhr commented Jul 6, 2024

See https://github.com/jmbuhr/quarto-nvim-kickstarter/blob/6455ad9123041f11e3294508db8f216223e2ca8a/lua/plugins/quarto.lua

  { -- requires plugins in lua/plugins/treesitter.lua and lua/plugins/lsp.lua
    -- for complete functionality (language features)
    'quarto-dev/quarto-nvim',
    ft = { 'quarto' },
    dev = false,
    opts = {},
    dependencies = {
      -- for language features in code cells
      -- configured in lua/plugins/lsp.lua and
      -- added as a nvim-cmp source in lua/plugins/completion.lua
      'jmbuhr/otter.nvim',
    },
  },

  { -- directly open ipynb files as quarto docuements
    -- and convert back behind the scenes
    'GCBallesteros/jupytext.nvim',
    opts = {
      custom_language_formatting = {
        python = {
          extension = 'qmd',
          style = 'quarto',
          force_ft = 'quarto',
        },
        r = {
          extension = 'qmd',
          style = 'quarto',
          force_ft = 'quarto',
        },
      },
    },
  },

@jmbuhr
Copy link
Owner

jmbuhr commented Jul 6, 2024

i.e. just use the Quarto filetype as I recommended in my previous answer.

@stevenxxiu
Copy link
Author

I could do that if I have to, but I'm just avoiding that as https://github.com/benlubas/molten-nvim/blob/v1.8.3/docs/Notebook-Setup.md#notebook-conversion says:

Jupytext can convert to the Quarto format, but it's slow enough to notice, on open and on save, so I prefer markdown

Another issue is that I'll have to consider both quarto and markdown filetypes everywhere in my config.

Is there no way to make it work with markdown files, while having a quarto config?

@jmbuhr
Copy link
Owner

jmbuhr commented Jul 6, 2024

In that case, I would advice to only activate otter manually with a keybinding. Markdown is used in many places in Neovim, including hover documentation windows, and you don't want all of them to behave like computational notebooks.

@jmbuhr
Copy link
Owner

jmbuhr commented Jul 6, 2024

Or like I said, find a way of activating otter automatically only after jupytext is completely done with the conversion.

@stevenxxiu
Copy link
Author

I tried to remove my config/after/ftplugin/markdown.lua and activate otter manually now with:

:lua require('otter').activate()

The same thing occurs. It doesn't update diagnostics after I edit the file.

@stevenxxiu
Copy link
Author

Ig I don't really need quarto anyway. I just realized the keybinds are gone now and got moved to LSP keybinds.

@00y300
Copy link

00y300 commented Jul 11, 2024

@stevenxxiu Were you able to get diagnostics working?

@stevenxxiu
Copy link
Author

stevenxxiu commented Aug 10, 2024

@00y300. When I last made a comment here, diagnostics updated sometimes but often didn't. I tried this again today. It's always worked so far. If it indeed got fixed, I'm not sure what fixed it. Maybe some plugin update or Neovim update.

I spoke too soon. Diagnostics only appear when the .ipynb file isn't empty when opened, and has one code block.

@jmbuhr
Copy link
Owner

jmbuhr commented Aug 10, 2024

Of course, otter can only activate for the languages present at the time of activating. If you add code chunks of a language not present when activating, simply activate again.

@stevenxxiu
Copy link
Author

stevenxxiu commented Aug 10, 2024

Makes sense. I updated my LuaSnip snippet to insert a code block, to activate otter. So otter now basically activates when I start coding in a Jupyter notebook.

s(
  ';C',
  fmta('```<>\n<>\n```', {
    f(function()
      local path = vim.api.nvim_buf_get_name(0)
      if string.match(path, '%.ipynb$') then
        local metadata = vim.json.decode(io.open(path, 'r'):read('a'))['metadata']
        local language = metadata.kernelspec.language

        vim.schedule(function()
          local otter = require('otter')
          local keeper = require('otter.keeper')

          local bufnr = vim.api.nvim_get_current_buf()
          local raft = keeper.rafts[bufnr]
          if raft ~= nil and raft.otterls.client_id ~= nil then
            return
          end
          local nb_language_map = {
            ['javascript'] = 'javascript',
            ['python'] = 'python',
            ['sage'] = 'python',
          }
          otter.activate({ nb_language_map[language] })
        end)

        return language
      end
      return ''
    end),
    i(1),
  })
),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants