search

eggbean / resize-font.gvim

gVim plugin for changing font/window size using keyboard or mousewheel.
MIT License
8 stars 2 forks source link

Handling things where gvim versions do not work with plugin "v1" and improvements #1

Open kennypete opened 9 months ago

kennypete commented 9 months ago

I like this, though there are some things that I think need hardening/fixing and some that will enhance it (most of which I have done, except part of point 5 and all of point 7). I have it ready for a PR, but did not want to send it unannounced with so many changes - here is my updated forked version.

  1. Before v9.0 patch 1112 will not work on Windows - refer patch 9.0.1112: test_mswin_event() can hang. A solution is to default map <C-=>= for the "FontSizeMinus" function when using a Windows version before that. It is a little clunky, but does the job.
  2. In Windows, the <C-Scroll*> mappings will not work before v8.2 patch 5069, though they do no harm. Refer patch 8.2.5069: various warnings from clang on MS-Windows.
  3. In Neovim, has("GUI_running") returns 1 (at least in the version I use just for testing; I don't use Neovim other than for that). So, it's probably best to include a specific test for Neovim, i.e., has('neovim'), though it's only needed in the vimscript version because the new (see the point following) vim9script version's test will exclude it anyway. (NB: Neovim returns 801 for v:version.)
  4. Since it is only relevant to the Vim GUI, it will only ever be a Vim plugin (so, never Neovim). In that context, and with the majority of Vim users probably using version 9, using vim9script makes sense. So, that's what I've done. The simplest way to have both scripts running in parallel (for the few people who may use different GUI versions, including some older ones) is to have both change_font_size_vim9script.vim and change_font_size.vim test for version 8.2 with/without patch 4807. (Note that vim9script works before v9.0, not just where has('vim9script'), so a good cut-off point, I think, is patch 8.2.4807: processing key eveints in Win32 GUI is not ideal, because that patch fixed <C-{whatever}>. (Before that patch, <C-{whatever}> mappings do nothing).
  5. Use maparg to ensure users' existing mappings are not overwritten and use map <silent> <Plug> too. The problem for the vimscript version, however, is that <C-{whatever}> does not work in version 8.2 before patch 4807 (in Windows at least, as noted). I did some testing using <F10> to <F12>, but it seems pretty flaky in Windows, and I don't want to burn more time testing it. (Using this Win64 zip version), amongst others in determining the points above in relation to version capability.)
  6. Using has('gui_gtk') covers both 2 and 3, so is a simple refinement.
  7. Although it's not wrong to recommend putting the script(s) directly into the .vim/plugin or vimfiles\plugin directory, it would be better recommending using the packages approach (outlined at :h packages). That is, using .vim/pack/{whatever}/start (or .vim/pack/{whatever}/opt, and calling it from your .gvimrc, which is probably preferable in this instance too, especially if you use console and GUI. That's because it would mean you then only source the script(s) when using the GUI rather than relying on the test in the script itself to finish when using console Vim). I've not updated the README.md in relation to this, but you could consider it - check out this README of mine if you like.
  8. Incidental: I popped in a GIF of it working in Debian WSL. You may want to look at your GIF - it is huge, comparatively.

That's about it. It's your repository so, if you're okay with what I have done, and are happy for a PR, let me know (and I'll make it and you can consider the few loose ends like point 5 after that).

I love the concept, btw, having stuffed around changing font sizes in the past, so thanks for putting it out there. (I will be taking the vim9script into my .gvimrc and _gvimrc directly because I prefer to have fewer plugins, and this is a pretty short script so only a small addition really.)

kennypete commented 9 months ago

Sure, Jason, happy for you to do whatever with it - keep/amend away ☑️. I’ll send the PR tomorrow morning …

12 Dec 2023 at 21:17:

Hi, of course I would be happy to receive your PR. I've only been using vim for two or three years, so I don't know about a lot the stuff you've covered here, yet. I'm glad to have these improvements that I didn't know needed to be made.

Would you mind if I made my changes to your PR? I want to get that GitHub pair programming badge.

— Reply to this email directly, > view it on GitHub https://github.com/eggbean/resize-font.gvim/issues/1#issuecomment-1851500576> , or > unsubscribe https://github.com/notifications/unsubscribe-auth/APN2VDZNWD7UXYSA54L5BSDYJAHKNAVCNFSM6AAAAABAQTPCPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJRGUYDANJXGY> . You are receiving this because you authored the thread.> Message ID: > <eggbean/resize-font> .> gvim/issues/1/1851500576> @> github> .> com>

eggbean commented 9 months ago

(sorry, edited and reposted)

Hi, of course I would be happy to receive your PR. I've only been using vim for two or three years, so I don't know about a lot the stuff you've covered here, yet. I'm glad to have these improvements that I didn't know needed to be made.

Could I ask what your reasoning is for point 4? Why make a parallel script using vim9script? I've not learnt anything about vim9script yet.

Would you mind if I made my changes to your PR? I want to get that GitHub pair programming badge.

kennypete commented 9 months ago

vim9script because:

^^ I was tempted to suggest running with vim9script solely because I think you’ll find before 8.2.4807 the experience is iffy to the point of not bothering with it.  

Likewise, I’m fairly new to Vim, having only used it for <3 years. 

12 Dec 2023 at 21:28 :

Hi, of course I would be happy to receive your PR. I've only been using vim for two or three years, so I don't know about a lot the stuff you've covered here, yet. I'm glad to have these improvements that I didn't know needed to be made.

Could I ask what your reasoning is for point 4? Why make a parallel script using vim9script?

Would you mind if I made my changes to your PR? I want to get that GitHub pair programming badge.

— Reply to this email directly, > view it on GitHub https://github.com/eggbean/resize-font.gvim/issues/1#issuecomment-1851519667> , or > unsubscribe https://github.com/notifications/unsubscribe-auth/APN2VD33XAMWEFCVZF4AYLTYJAIUDAVCNFSM6AAAAABAQTPCPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJRGUYTSNRWG4> . You are receiving this because you authored the thread.> Message ID: > <eggbean/resize-font> .> gvim/issues/1/1851519667> @> github> .> com>