Removed 'let' from session file content

[JetSetIlly]

Reecent releases of vim specify Session files to be vim9script. 'let' is no longer allowed as the assignment operator in vim9script

This change is all that's needed for Obsession to function without warning messages in recent builds or vim. It would be further improved if the plugin could somehow discern that the session file is using vim9script. That way, people who are still using older versions of vim can still use the most recent version of the plugin.

However, I'm not sure how to do that best so I've not attempted it.

I raised the issue in the main vim repository vim/vim#20588

[tpope]

It would be further improved if the plugin could somehow discern that the session file is using vim9script. That way, people who are still using older versions of vim can still use the most recent version of the plugin.

"Older" as in a month or two. I don't even have any machines running the new version yet. Plus there's Neovim compatibility. This needs to be backwards compatible.

There's also a hard-coded check for the old-style file that needs to be addressed:

vim-obsession/plugin/obsession.vim

Line 40 in f50baec

\ && readfile(file, '', 1)[0] !=# 'let SessionLoad = 1'

That check in and of itself may offer a clue how to distinguish between the old format and new.

[JetSetIlly]

"Older" as in a month or two. I don't even have any machines running the new version yet. Plus there's Neovim compatibility. This needs to be backwards compatible.

I've added a check for the vim9script directive in the persist() function.

I've tested with an older version (from a couple of months ago)

1. A vimscript session file is produced. The let keyword is used
2. Running the current github version of vim successfully loads the vimscript session file
3. Quitting vim updates the session file. The vim9script directive is added. The let keyword is not used
4. Loading the new session file works as expected

Of course, loading a new session file with an older version of vim does not work. But that's expected and not something we can account for.

I don't have access to neovim so that remains untested.

[eight04]

Even with this fix, we still get an error when loading old session files, right?

I suspect this breaks other plugins (e.g. prosession) that relies on g:this_session and g:this_obsession because those variables weren't setup properly.

[eight04]

Didn't understand the logic but this might fix L40:

diff --git a/plugin/obsession.vim b/plugin/obsession.vim
index ef2a998..e63b2ef 100644
--- a/plugin/obsession.vim
+++ b/plugin/obsession.vim
@@ -37,7 +37,7 @@ function! s:dispatch(bang, file) abort
       \ && file !~# 'Session\.vim$'
       \ && filereadable(file)
       \ && getfsize(file) > 0
-      \ && readfile(file, '', 1)[0] !=# 'let SessionLoad = 1'
+      \ && !s:is_valid_session(file)
       return 'mksession '.fnameescape(file)
     endif
     let g:this_obsession = file
@@ -54,6 +54,18 @@ function! s:dispatch(bang, file) abort
   endtry
 endfunction
 
+function! s:is_valid_session(file) abort
+  let lines = readfile(a:file, '', 2)
+  if empty(lines)
+    return 0
+  endif
+  if lines[0] ==# 'vim9script' && len(lines) > 1
+    return lines[1] ==# 'g:SessionLoad = 1'
+  else
+    return lines[0] ==# 'let SessionLoad = 1'
+  endif
+endfunction
+
 function! s:doautocmd_user(arg) abort
   if !exists('#User#' . a:arg)
     return ''

(Won't mksession fail with EEXIST?)

[sodapopcan]

So I went and opened #88 not seeing this one 🤦

My strategy was to just check for has('patch-9.2.0579') which is where this was introduced. This is backward compatible with Vim and should work with NeoVim since it would make no sense for it to include this patch. However you could be "extra safe" with an added && !has('nvim').

I agree that supporting opening a vim9script Session in an older version doesn't make sense as Vim itself doesn't worry about that.

The check for let SessionLoad = 1 could simply be:

&& readfile(file, '', 2)[0] !=# '\%(let\s\|g:\)SessionLoad = 1' 

EDIT: Disregard the SessionLoad stuff, I totally misinterpreted what was going on there (it also has syntax errors since I didn't test it!) What @eight04 is fine, although the check for empty(lines) should be removed as that is already handled earlier (getfsize call). It's possible the check for len(lines) isn't necessary either but I'm not sure.

Just to make myself feel better about my dumb code above, this one-liner actually works:

&& join(readfile(file, '', 2)) !~# '\%(let\s\|vim9script\sg:\)SessionLoad = 1' 

[trev-dev]

Guys you can just check `if has('vim9script')' and call it a day:

      if has('vim9script')
        call insert(body, 'g:this_session = v:this_session', -3)
        call insert(body, 'g:this_obsession = v:this_session', -3)
      else
        call insert(body, 'let g:this_session = v:this_session', -3)
        call insert(body, 'let g:this_obsession = v:this_session', -3)
      endif

Tested on my local, works fine. I don't use Neovim anymore, but neovim doesn't have vim9script anyway. The vim9script feature check is what's suggested by the contribution guidelines at the vim repository for ensuring Neovim compatibility. See: https://github.com/vim/vim/blob/master/CONTRIBUTING.md

Related: #89

[tpope]

Guys you can just check `if has('vim9script')' and call it a day

This would inject invalid vim9script syntax into legacy syntax sessions generated by Vim < 9.2.0579.

[trev-dev]

@tpope like, if you had a really old Session file? Thanks for the heads up and the work.

[sodapopcan]

Since I hearted your answer thinking "Oh ya, duh!" then realized "oh wait..." it's that this issue wasn't due to vim9script in general, it's that Session files were converted to vim9script. There are four years worth of Vim versions that include vim9script but session files were being written as "legacy" script.

[trev-dev]

Yep, that's a gap I definitely missed