10000 sessions: do not save mappings defined in vim9script context by habamax · Pull Request #16738 · vim/vim · GitHub
[go: up one dir, main page]
Skip to content

sessions: do not save mappings defined in vim9script context #16738

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
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

sessions: do not save mappings defined in vim9script context #16738

wants to merge 4 commits into from
+58 −1

Conversation

habamax
Copy link
Contributor
@habamax habamax commented Feb 26, 2025

@chrisbra
Copy link
Member

Hm, that sounds a bit brutal. If someone uses vimrc usig vim9script all mappings won't be restored? Wouldn't it be cleaner to add packadd calls for any mappings that come from an (optional) package? Assuming start packages already work correctly.

@habamax
8000 Copy link
Contributor Author
habamax commented Feb 26, 2025

Hm, that sounds a bit brutal. If someone uses vimrc usig vim9script all mappings won't be restored? Wouldn't it be cleaner to add packadd calls for any mappings that come from an (optional) package? Assuming start packages already work correctly.

I don’t think it would, the issue is not in optional packages but in the way session is saved and restored. Adding packadd will do nothing for a mapping that uses function coming from vim9script import.

@habamax
Copy link
Contributor Author
habamax commented Feb 26, 2025

There might be a better way, but I can’t see it. Check #16688

@chrisbra
Copy link
Member

In any case, can you please add a test for this?

@chrisbra
Copy link
Member

I'd like to hear others opinions.

@habamax
8000
Copy link
Contributor Author
habamax commented Feb 27, 2025

In any case, can you please add a test for this?

Let's wait for other opinions and if it would be "a go", will add test(s)

@habamax
Copy link
Contributor Author
habamax commented Feb 28, 2025 •
edited
Loading

Hm, that sounds a bit brutal. If someone uses vimrc usig vim9script all mappings won't be restored?

What I was thinking with this implementaion/fix:

  1. Session.vim is essentially non-vim9script file,
  2. mappings defined in vim9script context might work but it is not guaranteed,
  3. "most probably" mappings, one have in a package or in vimrc, doesn't make sense to persist as they "most of the time" would be restored from the vimrc or a package and
  4. temporary mappings, the ones user creates with command line, "most probably" would be in legacy vimscript context and thus they will be saved and restored with a session.

@habamax
Copy link
Contributor Author
habamax commented Feb 28, 2025

added 2 tests: for mappings defined in and out of vim9script context.

@habamax habamax closed this Jun 10, 2025
@habamax habamax deleted the session-vim9-map branch June 10, 2025 00:22
@habamax
Copy link
Contributor Author
habamax commented Jun 10, 2025

Oh, I've forgot branch deletion would close the pr too.

@chrisbra should I restore it?

@habamax habamax restored the session-vim9-map branch June 10, 2025 03:49
@habamax habamax reopened this Jun 10, 2025
@chrisbra
Copy link
Member

Ah thanks. I didn't notice earlier. Let's leave it open for now. Thanks

@habamax habamax mentioned this pull request Jun 12, 2025
Closed
@kennypete
Copy link
Contributor

Fixing sessions from breaking <ScriptCmd> mappings like, map <Plug>(ScriptCmd_DoIt) <ScriptCmd>ScriptCmd.DoIt()<CR> ... yes.

I was about to create an issue today, having been driven crazy and wasting loads of time debugging why a plugin's <ScriptCmd> mapping was failing with E492, then found Issue 16688 and this PR.

Workaround? An autocmd to remove <ScriptCmd> lines from the session:

autocmd SessionWritePost * FixSessionFile()
def FixSessionFile(): void
  try
    v:this_session
      ->readfile('b')
      ->filter('v:val !~ "<ScriptCmd>"')
      ->writefile(v:this_session, 'b')
  catch
    echo v:exception
  endtry
enddef

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Apakottur Apakottur Apakottur approved these changes

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

Successfully merging this pull request may close these issues.

Session restoration breaks mappings Comment plugin loses state when Session loads
4 participants
@habamax @chrisbra @kennypete @Apakottur
0