Add gui support to getcellpixels by mikoto2000 · Pull Request #16059 · vim/vim

mikoto2000 · 2024-11-15T02:45:19Z

#16004 and #16047 only supported Unix terminal, but this time we added support for gVim.

src/testdir/test_functions.vim

ychin · 2024-11-15T04:38:21Z

src/evalfunc.c

+    {
+#endif
+        struct cellsize cs;
+        mch_calc_cell_size(&cs);


I think it might be easier if you just do something like this, and don't define this function in os_win32.c:

#if defined(UNIX) mch_calc_cell_size(&cs); #else cs_out->cs_xpixel = -1; cs_out->cs_ypixel = -1; #endif

The reason is that outside of Unix and Win32 we have platform definitions for things like VMS / Amiga. They aren't used that often or checked in CI, but unless you define this function for all platforms, this code won't build on all of them. E.g. if you look at mch_get_shellsize for example, it's defined for Amiga, VMS, etc. I'm not sure if you want to define all of them just to have them set xpixel to -1.

Fixed in 44b5e6c

ychin · 2024-11-16T22:51:21Z

Random things I found in getcellpixels():

On macOS, Terminal.app (the default system terminal) returns the results in points / virtual pixels, which are the screen-space coordinates before Retina Display's 2x pixel scaling (Apple generally prefers applications to work in pre-scaling resolutions and let the APIs "just work", unlike how on Linux/Windows where the raw pixels tend to be exposed even with 1.5x/2x scaling). However, other macOS terminals like Kitty and iTerm2 returns raw pixels which means they have the 2x scaling applied to them. This leads to potentially confusing results on macOS. Not sure there's anything we could do here other than clarifying it in the docs.
The default Gnome Terminal returns [0,0] for this API, but other terminals (e.g. Kitty) seem to work just fine. :/

For (2), I am not sure if we can do anything about the Gnome terminal part (just let the user deal with it). We could potentially just return [] in that case (if we detect the pixel values are 0) but it would break our tests right now since we are actually passing [0,0] to the embedded Vim terminal anyway… We could just leave it as is? Otherwise it may need more testing to see how this behaves in different terminals.

For (1), perhaps we could clarify it in the docs. Something like this (feel free to edit)?

getcellpixels()						*getcellpixels()*
		Returns a |List| of terminal cell pixel size.
		List format is [xpixels, ypixels].
        Only works on Unix (terminal and gVim) and Windows (gVim only).
        Returns [] on other systems or on failure.
        Note that there could be variations across different terminals. On
        macOS, system Terminal.app returns sizes in points (before Retina
        scaling), whereas third-party terminals return raw pixel sizes (post
        Retina scaling).

We could fix the docs up in a future PR as well if you don't want this PR to balloon.

chrisbra · 2024-11-17T14:58:41Z

@mikoto2000
Can you please squash this one in as well please?

commit 88ea90954b828bbb655156ba239514c454ccb19a (HEAD -> test_term_assert)
Author: Christian Brabandt <[email protected]>
Date:   Sun Nov 17 09:52:30 2024 +0100

    floating point exception in GVim

    Problem:  floating point exception in GVim
    Solution: Return early when returned pixel size is 0

    Signed-off-by: Christian Brabandt <[email protected]>

diff --git a/src/os_unix.c b/src/os_unix.c
index 80ca0ce26..3e9f5754e 100644
--- a/src/os_unix.c
+++ b/src/os_unix.c
@@ -4391,7 +4391,7 @@ mch_calc_cell_size(struct cellsize *cs_out)
    ch_log(NULL, "ioctl(TIOCGWINSZ) %s", retval == 0 ? "success" : "failed");
 #endif

-   if (retval == -1)
+   if (retval == -1 || ws.ws_xpixel == 0)
    {
        cs_out->cs_xpixel = -1;
        cs_out->cs_ypixel = -1;

This causes the failure in GitHub CI / linux (huge, gcc, uchar, testgui, true, dynamic) (pull_request)

See also #16072

I commented on #16072 (comment) but I think this change will break the existing getcellpixels() test since it relies on passing a 0 x/y pixels to the hosted terminal since in CI we don't know what the pixel size is as ioctl call would fail.

k-takata · 2024-11-18T09:18:23Z

src/evalfunc.c

+#if defined(FEAT_GUI)
+    if (gui.in_use)
+    {
+        // success pixel size and no gui.


The comment "no gui" looks wrong.

Problem: getcellpixels() can be further improved Solution: Fix floating point exception, implement getcellpixels() in the UI (mikoto2000) closes: vim#16059 Signed-off-by: mikoto2000 <[email protected]> Signed-off-by: Christian Brabandt <[email protected]>

CI: Bump codecov/codecov-action from 4 to 5 Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 4 to 5. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v4...v5) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-type: direct:production update-type: version-update:semver-major ... closes: vim#16079 Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0870: too many strlen() calls in eval.c Problem: too many strlen() calls in eval.c Solution: Refactor eval.c to remove calls to STRLEN() (John Marriott) closes: vim#16066 Signed-off-by: John Marriott <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0871: getcellpixels() can be further improved Problem: getcellpixels() can be further improved Solution: Fix floating point exception, implement getcellpixels() in the UI (mikoto2000) closes: vim#16059 Signed-off-by: mikoto2000 <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0872: No test for W23 message Problem: No test for W23 message Solution: Check for W23 message when accessing the clipboard fails (after v9.1.0868) closes: vim#16076 Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0873: filetype: Vivado files are not recognized Problem: filetype: Vivado files are not recognized Solution: detect '*.mss' files as 'mss' filetype (Wu, Zhenyu) references: https://docs.amd.com/r/2020.2-English/ug1400-vitis-embedded/Microprocessor-Software-Specification-MSS closes: vim#15907 Signed-off-by: Wu, Zhenyu <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> CI: join codecov array flags, instead of accessing it directly closes: vim#16082 Signed-off-by: rhysd <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0874: filetype: karel files are not detected Problem: filetype: karel files are not detected Solution: detect '*.kl' files as karel filetype, include syntax and filetype plugin (Kirill Morozov) closes: vim#16075 Co-authored-by: KnoP-01 <[email protected]> Signed-off-by: Kirill Morozov <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0875: filetype: hyprlang detection can be improved Problem: filetype: hyprlang detection can be improved Solution: detect '/hypr/*.conf' files as hyprlang filetype, include basic syntax highlighting (Luca Saccarola) fixes: vim#15875 closes: vim#16064 Signed-off-by: Luca Saccarola <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0876: filetype: openCL files are not recognized Problem: filetype: openCL files are not recognized Solution: detect '*.cl' files as opencl or lisp filetype, include a opencl syntax and filetype plugin (Wu, Zhenyu) closes: vim#15825 Signed-off-by: Wu, Zhenyu <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0877: tests: missing test for termdebug + decimal signs Problem: tests: missing test for termdebug + decimal signs Solution: Add a termdebug test (Ubaldo Tiberi) closes: vim#16081 Signed-off-by: Ubaldo Tiberi <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0878: termdebug: cannot enable DEBUG mode Problem: termdebug: cannot enable DEBUG mode Solution: Allow to specify DEBUG mode (Ubaldo Tiberi) closes: vim#16080 Signed-off-by: Ubaldo Tiberi <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> runtime(compiler): fix escaping of arguments passed to :CompilerSet See newly added help entry referring to option-backslash closes: vim#16084 Signed-off-by: Konfekt <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> Add clang-format config file This is used in preparation to enable automatic code-formatting in the following commits. For now let's just add a clang-format config file, formatting of source files will follow. related: vim#16019 Signed-off-by: Luca Saccarola <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0879: source is not consistently formatted Problem: source is not consistently formatted Solution: reformat sign.c and sound.c (Luca Saccarola) closes: vim#16019 Signed-off-by: Luca Saccarola <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> git: ignore re-formatting commit v9.1.0879 for blame Signed-off-by: Christian Brabandt <[email protected]> runtime(doc): add helptag for :HelpToc command Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0880: filetype: C3 files are not recognized Problem: filetype: C3 files are not recognized Solution: detect '*.c3*' files as c3 filetype (Turiiya) closes: vim#16087 Signed-off-by: Turiiya <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> runtime(misc): add Italian LICENSE and (top-level) README file related: vim#16061 Signed-off-by: Antonio Giovanni Colombo <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> runtime(apache): Update syntax keyword definition closes: vim#16105 Signed-off-by: nisbet-hubbard <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> runtime(netrw): update netrw's decompress logic Detect a few more default archive types, correctly handle file extensions with digits in it. fixes: vim#16099 closes: vim#16104 Signed-off-by: Konfekt <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0881: GUI: message dialog may not get focus Problem: GUI: message dialog may not get focus Solution: add window manager hint to give focus to the dialog (Chris White) Tell the window manager that message dialogs should be given focus when the user switches from another application back to Vim. This can happen, e.g., when the user has a file open in Vim and then edits it in another program. fixes: vim#172 closes: vim#16100 Signed-off-by: Chris White <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> patch 9.1.0882: too many strlen() calls in insexpand.c Problem: too many strlen() calls in insexpand.c Solution: Refactor insexpand.c and reduce number of calls to STRLEN(), fix a warning get_next_filename_completion(), add new function ins_compl_leader_len() (John Marriott) closes: vim#16095 Signed-off-by: John Marriott <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> runtime(netrw): Fixing powershell execution issues on Windows closes: vim#16094 Signed-off-by: GuyBrush <[email protected]> runtime(doc): Expand docs on :! vs. :term fixes: vim#16071 closes: vim#16089 Signed-off-by: matveyt <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> rename the files of the chapter one to tutor1 The numbering of lessons in the files of chapter one begins with 1 the files of chapter one

mikoto2000 added 5 commits November 15, 2024 01:14

Added windows function.

40575f4

Add support on gui.

11cf247

Move cellsize struct.

f0fe415

Fixed mch_calc_cell_size in os_win32.c.

7dccad5

Add test.

1a0dc7d

github-actions bot added the platform-windows label Nov 15, 2024

zeertzjq reviewed Nov 15, 2024

View reviewed changes

src/testdir/test_functions.vim Outdated Show resolved Hide resolved

src/testdir/test_functions.vim Outdated Show resolved Hide resolved

src/testdir/test_functions.vim Outdated Show resolved Hide resolved

ychin reviewed Nov 15, 2024

View reviewed changes

mikoto2000 added 3 commits November 15, 2024 04:47

Fix expected value in getcellpixels on gVim.

112ae31

Delete the incorrect comment.

e23f96a

Remove mch_calc_cell_size in os_win32.c.

44b5e6c

mikoto2000 added 2 commits November 17, 2024 23:57

floating point exception in GVim

2cca5a6

Problem: floating point exception in GVim Solution: Return early when returned pixel size is 0 Signed-off-by: Christian Brabandt <[email protected]>

Updated the documentation for getcellpixels().

a977330

Added information about Retina scaling.

k-takata reviewed Nov 18, 2024

View reviewed changes

Fixed ifdefs style.

7818a49

k-takata reviewed Nov 18, 2024

View reviewed changes

chrisbra closed this in a73dfc2 Nov 18, 2024

+                      // Non-Unix CUIs are not supported, so set this to -1x-1.
+                      cs.cs_xpixel = -1;
+                      cs.cs_ypixel = -1;
+              # endif

	# else
	#else

Uh oh!

Conversation

mikoto2000 commented Nov 15, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ychin Nov 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ychin commented Nov 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbra commented Nov 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikoto2000 commented Nov 17, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k-takata Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ychin commented Nov 18, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ychin Nov 15, 2024 •

edited

Loading

ychin commented Nov 16, 2024 •

edited

Loading

chrisbra commented Nov 17, 2024 •

edited

Loading

k-takata Nov 18, 2024 •

edited

Loading