Skip to content
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.

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

If wchar is not included in ncurses, mc 4.8.26 fails to compile. #4200

Closed
mc-butler opened this issue Feb 13, 2021 · 13 comments
Closed

If wchar is not included in ncurses, mc 4.8.26 fails to compile. #4200

mc-butler opened this issue Feb 13, 2021 · 13 comments
Labels
area: tty Interaction with the terminal, screen libraries prio: low Minor problem or easily worked around ver: 4.8.26 Reproducible in version 4.8.26
Milestone

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/4200
Reporter mrbump (rudi@….com)

With the introduction of shadows in mc (which are nice) on platforms that support them with the following commit - https://github.com/MidnightCommander/mc/commit/8b4386df83ab5a525f0568113fe1e53d362f433e.patch and Ticket #4120: draw shadows for dialog boxes and menus.

Mc 4.8.26 fails to compile on LibreELEC. - https://libreelec.tv/ - the Tagline being Just enough OS for KODI

We use --disable-widec in the ncurses build, https://github.com/LibreELEC/LibreELEC.tv/blob/master/packages/devel/ncurses/package.mk

Reverting the whole patch fixes the compile, but it would be ideal to use alternatives or limit shadows if wchar is not available.

Results of the failed compile

./.libs/libinternal.a(tty-ncurses.o):tty-ncurses.c:function tty_colorize_area: error: undefined reference to 'mvin_wchnstr'
./.libs/libinternal.a(tty-ncurses.o):tty-ncurses.c:function tty_colorize_area: error: undefined reference to 'getcchar'
./.libs/libinternal.a(tty-ncurses.o):tty-ncurses.c:function tty_colorize_area: error: undefined reference to 'setcchar'
./.libs/libinternal.a(tty-ncurses.o):tty-ncurses.c:function tty_colorize_area: error: undefined reference to 'mvadd_wchnstr'

Note

Original attachments:

@mc-butler
Copy link
Author

Changed by mrbump (rudi@….com) on Feb 13, 2021 at 11:43 UTC (comment 1)

The code that has the wchar is

+++ b/lib/tty/tty-ncurses.c
@@ -533,6 +550,38

/* --------------------------------------------------------------------------------------------- */

+void
+tty_colorize_area (int y, int x, int rows, int cols, int color)
+{
+    cchar_t *ctext;
+    wchar_t wch[10];   /* TODO not sure if the length is correct */
+    attr_t attrs;
+    short color_pair;
+
+    if (!use_colors || !tty_clip (&y, &x, &rows, &cols))
+        return;
+
+    tty_setcolor (color);
+    ctext = g_malloc (sizeof (cchar_t) * (cols + 1));
+
+    for (int row = 0; row < rows; row++)
+    {
+        mvin_wchnstr (y + row, x, ctext, cols);
+
+        for (int col = 0; col < cols; col++)
+        {
+            getcchar (&ctext[col], wch, &attrs, &color_pair, NULL);
+            setcchar (&ctext[col], wch, attrs, color, NULL);
+        }
+
+        mvadd_wchnstr (y + row, x, ctext, cols);

@mc-butler
Copy link
Author

Changed by mrbump (rudi@….com) on Feb 13, 2021 at 11:56 UTC (comment 2)

Original change that created the issue #4102

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 13, 2021 at 13:02 UTC (comment 3)

  • Component changed from mc-core to mc-tty
  • Resolution set to invalid
  • Status changed from new to closed
  • Milestone Future Releases deleted

Fixed as #4181.

@mc-butler
Copy link
Author

Changed by mrbump (rudi@….com) on Feb 14, 2021 at 0:50 UTC (comment 4)

Whilst that patch fixed MacOS it doesn’t fix LibreELEC as wide char is not included. I had tested that patch as I had compiled against master https://github.com/MidnightCommander/mc/commits/master . If you could suggest a direction of what would be acceptable to the mc team, I would be happy to develop a code fix. As in tty-slang I see that tty_colorize_area is essentially a noop.

Should I write a —without-wchar configure option?

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Feb 14, 2021 at 10:12 UTC (comment 5)

How is it possible that LibreELEC doesn't have a definition of wchar_t? Kodi itself seems to use it all over the place... How do you solve this problem?

To me it seems that of course you do have wchar_t, only for some reason we cannot understand yet you compile ncurses with --disable-widec, so even though mc does compile against ncurses, according to your log the linking fails, because mvadd_wchnstr function cannot be found. Just compile ncurses --enable-widec to solve this problem if you can. I would be surprised if mc interface will be rendered correctly if you compile against ncurses with --disable-widec anyways.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 15, 2021 at 17:40 UTC

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 15, 2021 at 17:40 UTC (comment 6)

Please test the attached patch.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 28, 2021 at 14:02 UTC (comment 7)

ping

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Mar 7, 2021 at 15:56 UTC (comment 8)

@mrbump, please let me know if this patch works or not.

@mc-butler
Copy link
Author

Changed by mrbump (rudi@….com) on Aug 23, 2021 at 13:49 UTC (comment 9)

Hi @andrew_b

we just pulled the patch into 4.8.27. And removed the previous revert patch. All works as expected. LibreELEC/LibreELEC.tv#5562

Regards
Rudi

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Aug 28, 2021 at 8:47 UTC (comment 10)

  • Status changed from closed to reopened
  • Milestone set to 4.8.28
  • Branch state changed from no branch to on review
  • Resolution invalid deleted

Branch: 4200_ncurses_disable_widec
[e7bbf72]

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Aug 28, 2021 at 8:49 UTC (comment 11)

  • Votes set to mrbump andrew_b
  • Branch state changed from on review to approved

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Aug 28, 2021 at 8:50 UTC (comment 12)

  • Resolution set to fixed
  • Votes changed from mrbump andrew_b to committed-master
  • Branch state changed from approved to merged
  • Status changed from reopened to closed

Merged to master: [375f2a7].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: tty Interaction with the terminal, screen libraries prio: low Minor problem or easily worked around ver: 4.8.26 Reproducible in version 4.8.26
Development

No branches or pull requests

1 participant