Skip to content

RFC: Add session option - #27

Open
laburnumT wants to merge 2 commits into
gpanders:masterfrom
laburnumT:master
Open

RFC: Add session option#27
laburnumT wants to merge 2 commits into
gpanders:masterfrom
laburnumT:master

Conversation

@laburnumT

Copy link
Copy Markdown

Adds a session option to codeblocks so previous results can be used.

To check that a command is done running, we wait for an eof_token to appear. This allows us to not cut off commands with output early, but also doesn't rely on using sleep.

@laburnumT

Copy link
Copy Markdown
Author

This is an RFC for adding session support to vim-medieval. It's currently quite
hacky, because repl's aren't always accessed the same way. But that's why it's a
first draft.

It currently has some checks in there for python (because that's what I use in
markdown for the most part).

This should probably be integrated into g:medieval_langs, or a new config
variable, but I just wanted to know if there is any interest in functionality
like this before I do that work.

@laburnumT

Copy link
Copy Markdown
Author

The main problem this is trying to address is where something is either costly
to run or not idempotent. As a rather stupid example:

<!-- session: example -->
```python
import random
import time

time.sleep(5)
x = 42
```

<!-- session: example -->
```python
print(random.randint(1, x))
```

Using require here would make the print block always take 5 seconds, whereas
with a session it would run instantly (as long as the first code block has been
run at least once)

laburnumT added 2 commits July 9, 2026 09:59
Adds a session option to codeblocks so previous results can be used.

To check that a command is done running, we wait for an eof_token to
appear. This allows us to not cut off commands with output early, but
also doesn't rely on using sleep.

**This commit does not support neovim**
Splits up retrieving the data from s:session_read_cb into separate
helper functions for vim and nvim.
@gpanders

Copy link
Copy Markdown
Owner

The main problem this is trying to address is where something is either costly to run or not idempotent. As a rather stupid example:

<!-- session: example -->
```python
import random
import time

time.sleep(5)
x = 42
print(random.randint(1, x))

Using require here would make the print block always take 5 seconds, whereas with a session it would run instantly (as long as the first code block has been run at least once)

The use case makes sense to me. Though I think you should already be able to solve this today with no changes.

For example, something like this:

<!-- target: a -->
```python
import random
import time

time.sleep(5)
x = 42
print(f"x={x}")
```

<!-- name: a
```python
```
-->

<!-- require: a -->
```python
import random
print(random.randint(1, x))
```

I understand this is a contrived example, but the basic idea is: in the first (expensive) block, write code to a temporary block (a in this case) that re-uses the expensive results of the computation. Then you can require that temporary block in the final block.

Note that the a block is wrapped inside the HTML comment so that it won't be visible in any rendered HTML output.

@laburnumT

Copy link
Copy Markdown
Author

That would probably solve a lot of cases. But I feel like it might run into
problems with something like Haskell or some other language. But even in python
this does run into problems with classes that aren't printable (don't have a
dedicated str method).

Something like the following wouldn't work:

<!-- target: a -->
```python
class Example:
    def __init__(self, a, b):
        self.a = a
        self.b = b


example = Example(1, 2)
print(f"example={example}")
```

<!-- name: a
```python
example=<__main__.Example object at 0x723d8c290d70>
```
-->

<!-- require: a, target: out -->
```python
print(example.a)
```

<!-- name: out -->
```python
  File "/tmp/vqFx8sb/13", line 1
    example=<__main__.Example object at 0x723d8c290d70>
            ^
SyntaxError: invalid syntax
```

It also seems rather cumbersome to have to add all the print statements one
might need.

@laburnumT

Copy link
Copy Markdown
Author

Apologies if you are on holiday or otherwise occupied. In which case please
ignore this.

Just wanted to see if there were any more thoughts on this RFC.

@gpanders gpanders left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

As I said earlier the use case makes sense to me and I'm not opposed to the feature on principle but the PR needs some work.

Most of the comments I left are easily actionable, just a few small refactors. But there are some missing design points still, in particular how to map the "session" paradigm to each individual language without hard coding special cases into the source code.

Comment thread autoload/medieval.vim
Comment on lines +305 to +321
function! s:vim_cb(channel, msg) abort
for [k, s] in items(s:active_sessions)
if s.id == a:channel
call s:session_read(k, a:msg)
break
endif
endfor
endfunction

function! s:nvim_cb(job_id, data, event) abort
for [k, s] in items(s:active_sessions)
if s.id == a:job_id
call s:session_read(k, a:data)
break
endif
endfor
endfunction

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

These two functions are identical, let's consolidate (name it s:sessiondata since it's only used for sessions)

Comment thread autoload/medieval.vim
call winrestview(view)
endfunction

function! s:session_read(key, lines) abort

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Follow the conventions of the repo:

Suggested change
function! s:session_read(key, lines) abort
function! s:sessionread(key, lines) abort

Comment thread autoload/medieval.vim
break
endif
endfor
endfunction

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why do you have an exit callback for Nvim but not Vim?

Let's rename this s:sessionexit and re-use it for both Nvim and Vim (use a lambda if needed, look at the existing pattern in s:jobstart as a reference)

Comment thread autoload/medieval.vim
endfor
endfunction

function! s:eval_session(lang, session_name, block, cb) abort

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
function! s:eval_session(lang, session_name, block, cb) abort
function! s:evalsession(lang, session_name, block, cb) abort

Comment thread autoload/medieval.vim
Comment on lines +354 to +356
if a:lang ==# 'python' || a:lang ==# 'python3'
let cmd += ['-i', '-q']
endif

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We're not going to hard code languages in here (we have to do it for cmd to work around Windows wonkiness and even then I tried to avoid it). Let's find a way to make this work that is language agnostic and doesn't require special cases.

Comment thread autoload/medieval.vim
Comment on lines +358 to +389
if has('nvim')
let id = jobstart(cmd, {
\ 'on_stdout': function('s:nvim_cb'),
\ 'on_stderr': function('s:nvim_cb'),
\ 'on_exit': function('s:nvim_session_exit_cb'),
\ 'stdout_buffered': 0,
\ 'stderr_buffered': 0,
\ })
if id <= 0
return s:error('Failed to start job for ' . a:lang)
endif
let s:active_sessions[key] = {
\ 'id': id,
\ 'buffer': [],
\ 'token': '',
\ 'context': {},
\ }
else
let job = job_start(l:cmd, {
\ 'out_cb': function('s:vim_cb'),
\ 'err_cb': function('s:vim_cb'),
\ 'mode': 'nl',
\ })
let s:active_sessions[key] = {
\ 'id': job_getchannel(job),
\ 'job': job,
\ 'buffer': [],
\ 'token': '',
\ 'context': {},
\ }
endif
endif

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Use exists('*jobstart') instead of has('nvim') (look at s:jobstart as a reference) and extract this entire block into a function s:sessionstart.

Comment thread autoload/medieval.vim
Comment on lines +397 to +401
if a:lang =~# 'python'
let new_block += ['print("' . eof_token . '")']
else
let new_block += ['echo "' . eof_token . '"']
endif

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Similar comment to earlier, we should not hard code language support here. And even then this is wrong, using echo for the else branch won't work.

Comment thread autoload/medieval.vim
Comment on lines +403 to +409
if has('nvim')
call chansend(session.id, new_block + [''])
else
for line in new_block
call ch_sendraw(session.id, line . "\n")
endfor
endif

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Follow the pattern of s:jobstart: make a function s:chansend that encapsulates the platform differences between Nvim and Vim and use exists('*chansend') instead of has('nvim')

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.

2 participants