cmatrix icon indicating copy to clipboard operation
cmatrix copied to clipboard

Version 3.0

Open ctrlcctrlv opened this issue 2 years ago • 30 comments

I do apologize for the massive number of changes in a single commit; you see, I had this on my hard drive for a few years, and just worked on it a bit here and there, recompiling it occasionally.

Someone on Twitter made me think about private branches of free software, after which I endeavored to give my branch back, because I noted it has far more features than the public branch.

Included:

  • OpenType font of mtx.pcf
  • User configurable UTF-8 character sets, emoji OK
  • Better rainbow mode
  • Better color switching with less reliance on bold to have lighter colors
  • Optional Gettext support
  • CMake bug fixes: actually checks if wresize, resizeterm available
  • Can force install X font and OTF font
  • Much of the code was made easier to read
  • Lock mode now shows a helpful message if user tries to kill cmatrix
  • Added -i which inverts colors of headings

I once again apologize for the absurd size of this PR.

ctrlcctrlv avatar Oct 30 '21 17:10 ctrlcctrlv

Related to #136

MrBrain295 avatar Oct 30 '21 18:10 MrBrain295

@ctrlcctrlv This is a great contribution, thank you!

I'm not a maintainer, but I'll review what I can to ease the burden of the large PR.

I'm curious where the mtx.otb font came from? It's a bit hard to inspect an opaque binary. I presume it's a font, but from a security perspective, it'd be nice to validate that it's safe.

Erotemic avatar Oct 30 '21 19:10 Erotemic

@Erotemic I'm mainly a libre font developer, it's the output of my work, bitmapfont2otb, run against the existing mtx.pcf.gz.

I then took the output and slightly tweaked the Unicode cmap table so that it would work as expected with the strange encoding expected.

It's certainly safe.

ctrlcctrlv avatar Oct 30 '21 20:10 ctrlcctrlv

Thank you for this pull request. This is indeed a massive change. I will look at it and get back soon.

abishekvashok avatar Nov 01 '21 14:11 abishekvashok

Wow, @ctrlcctrlv , that looks great. However, I cannot get it to compile on macos, although the public version does so without problems.

Using the first method with autoreconf - configure does not get very far, running autoreconf -i results in an error:

configure.ac: warning: AM_GNU_GETTEXT is used, but not AM_GNU_GETTEXT_VERSION or AM_GNU_GETTEXT_REQUIRE_VERSION configure.ac:5: warning: 'AM_CONFIG_HEADER': this macro is obsolete. configure.ac:5: You should use the 'AC_CONFIG_HEADERS' macro instead. ./lib/autoconf/general.m4:2434: AC_DIAGNOSE is expanded from... aclocal.m4:4030: AM_CONFIG_HEADER is expanded from... configure.ac:5: the top level configure.ac:28: warning: The macro `AC_HEADER_STDC' is obsolete. configure.ac:28: You should run autoupdate. ./lib/autoconf/headers.m4:704: AC_HEADER_STDC is expanded from... configure.ac:28: the top level configure.ac:32: warning: The macro `AC_TYPE_SIGNAL' is obsolete. configure.ac:32: You should run autoupdate. ./lib/autoconf/types.m4:776: AC_TYPE_SIGNAL is expanded from... configure.ac:32: the top level configure.ac:173: warning: AC_OUTPUT should be used without arguments. configure.ac:173: You should run autoupdate. configure.ac:7: installing './compile' configure.ac:7: installing './config.guess' configure.ac:7: error: required file './config.rpath' not found configure.ac:7: installing './config.sub' configure.ac:6: installing './install-sh' configure.ac:6: installing './missing' configure.ac: error: AM_GNU_GETTEXT used but SUBDIRS not defined Makefile.am: installing './depcomp' autoreconf: error: automake failed with exit status: 1

I run macos Big Sur (11.6) and autoreconf version 2.71 installed via homebrew. I have gettext installed (version 0.21) via homebrew.

The second method with cmake generates a configure script, but then I immediately run into the following compile error when doing make:

/Users/phytzel/Downloads/cmatrix-3/cmatrix.c:411:13: error: expected expression
            char* next;
            ^
/Users/phytzel/Downloads/cmatrix-3/cmatrix.c:412:38: error: use of undeclared identifier 'next'
            update = strtol(optarg, &next, 10);
                                     ^
/Users/phytzel/Downloads/cmatrix-3/cmatrix.c:413:18: error: use of undeclared identifier 'next'
            if (*next != '\0') c_die(INVALIDUPDATE_MSG);
                 ^

cmake is installed (version 3.21.4) via homebrew, automake is version 1.16.5, also installed via homebrew.

Nithilher avatar Nov 01 '21 16:11 Nithilher

I only tested building with cmake I'm afraid, and am unsure why both are even there, they're incompatible in other ways as well

So do e.g. cmake -B build

ctrlcctrlv avatar Nov 01 '21 18:11 ctrlcctrlv

@abishekvashok yes indeed, I'm sorry, for some context as to how this happened basically I used cmatrix to practice C while helping maintain FontForge in 2019…so I tinkered with it a lot over years…then I was reminded by a Twitterer that my version is probably very diverged but when I saw how much I figured you'd want some, if not all, of the work back.

ctrlcctrlv avatar Nov 02 '21 08:11 ctrlcctrlv

Thanks, @ctrlcctrlv. Using cmake results in another error, which I can avoid if I force it to use homebrew installed gcc instead of clang. However, linking fails in the end due to undefined symbols from libintl. I found that this sort of problem seems to happen in similar circumstances when one tries to compile code directly from a github repository, instead of released tarballs. I could resolve it: Linking by hand with -L/usr/local/lib -lintl works.

Nithilher avatar Nov 02 '21 09:11 Nithilher

yes indeed, I'm sorry,

No need to be sorry, I am happy with how much things are in this pr. Thank you for it. I will try to get this reviewed/possibly merged by end of the week.

abishekvashok avatar Nov 02 '21 10:11 abishekvashok

@Nithilher I disabled libintl by default as we don't have any translations yet anyway :sweat_smile:

ctrlcctrlv avatar Nov 04 '21 08:11 ctrlcctrlv

-c causes black characters on macOS. Also black background on the characters. The v2.0 release shows transparent background on the characters.

sarensabertooth avatar Nov 15 '21 10:11 sarensabertooth

-c enables CHARSET_KATAKANA in this version, it's quite different.

ctrlcctrlv avatar Nov 15 '21 15:11 ctrlcctrlv

Also, quick question - how hard would be it for you to stash the changes to various flags and keep existing ones the same? Quite hard, due to the way this was developed, as a private version I worked on over a lot of time and never had any intention at first of distributing, but then changed my mind.

My flag changes were all made with good reasons, I'd rather discuss the ones you don't agree with one by one. For example, the change to -c now reflects the documentation saying it will use Japanese. This is completely a lie, it showed symbols between U+3000 and U+303F:

image

CJK Punctuation. Yes, used in Japan, but not close to half-width katakana used in the Matrix movie. Author must not have been familiar with Japanese language.

ctrlcctrlv avatar Dec 25 '21 05:12 ctrlcctrlv

By the way I added flag -S so users could get the legacy behavior without having to learn the new user-defined charset system. I think documentation is the right answer, not adherence to buggy behavior :)

ctrlcctrlv avatar Dec 25 '21 05:12 ctrlcctrlv

@abishekvashok Any news on this getting merged and released ?

blackjak231 avatar Feb 28 '22 12:02 blackjak231

Hi, I'm trying to build with the Makefile generated with cmake -B build, but when running make it's giving me this error:

[ 50%] Building C object CMakeFiles/cmatrix.dir/cmatrix.c.o
/home/user/Projects/cmatrix/cmatrix/cmatrix.c:344:30: error: ‘LOCALEDIR’ undeclared (first use in this function)
  344 |     bindtextdomain (PACKAGE, LOCALEDIR);
      |                              ^~~~~~~~~
/home/user/Projects/cmatrix/cmatrix/cmatrix.c:344:30: note: each undeclared identifier is reported only once for each function it appears in
make[2]: *** [CMakeFiles/cmatrix.dir/build.make:76: CMakeFiles/cmatrix.dir/cmatrix.c.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:83: CMakeFiles/cmatrix.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

Am I missing some dependency?

Hornobster avatar May 24 '22 20:05 Hornobster

Hey I can't seem to replicate the issue, @Hornobster , but we'll wait for a few days to check if anyone else is getting this. If not, I think we are ready to merge this @blackjak231

abishekvashok avatar Jun 20 '22 10:06 abishekvashok

The default supplied cmake config does indeed fail with the message @Hornobster is getting, I had to run it as cmake -B build -D USE_GETTEXT=ON.

dithpri avatar Jun 20 '22 16:06 dithpri

I got the error @Hornobster got and also some warnings which meant functions from libintl.h was used without explicit declaration, so I modified the code like the patch below and it worked for me. fix_gettext.patch.txt content of the patch:

diff --git a/cmatrix.c b/cmatrix.c
index 1fc0d32..e2a71f7 100644
--- a/cmatrix.c
+++ b/cmatrix.c
@@ -341,8 +341,10 @@ int main(int argc, char *argv[]) {
 
     srand((unsigned) time(NULL));
     setlocale(LC_ALL, "");
+#ifdef HAVE_LIBINTL_H
     bindtextdomain (PACKAGE, LOCALEDIR);
     textdomain (PACKAGE);
+#endif
 
     while ((optchr = getopt(argc, argv, "aAbBcfhilLnrosSmxkVM:u:U:C:t:")) != EOF) {
         switch (optchr) {
@@ -394,7 +396,11 @@ int main(int argc, char *argv[]) {
             locked = true;
             //if -M was used earlier, don't override it
             if (msg[0] == '\0') {
+#ifdef HAVE_LIBINTL_H
                 msg = (char*)gettext(LOCKED_MSG);
+#else
+                msg = (char*)LOCKED_MSG;
+#endif
             }
             break;
         case 'M':
@@ -407,11 +413,14 @@ int main(int argc, char *argv[]) {
             oldstyle = true;
             break;
         case 'u':
+            {
             char* next;
             update = strtol(optarg, &next, 10);
             if (*next != '\0') c_die(INVALIDUPDATE_MSG);
             update = min(update, 10);
             update = max(update, 0);
+
+            }
             break;
         case 'V':
             version();
diff --git a/util.h b/util.h
index e4aae7c..08586d4 100644
--- a/util.h
+++ b/util.h
@@ -14,7 +14,11 @@ void c_die(const char *msg, ...) {
     va_list ap;
     fprintf(stderr, "cmatrix: error: ");
     va_start(ap, msg);
+#ifdef HAVE_LIBINTL_H
     vfprintf(stderr, gettext(msg), ap);
+#else
+    vfprintf(stderr, msg, ap);
+#endif
     va_end(ap);
     fprintf(stderr, "\n");
     exit(EXIT_FAILURE);

ark231 avatar Jun 21 '22 07:06 ark231

@ark231 thanks for this diff, I will apply it to this PR test, and get back :)

abishekvashok avatar Jun 21 '22 09:06 abishekvashok

@ark231 Why the block in case 'u'?

polluks2 avatar Jun 23 '22 08:06 polluks2

@polluks2 It is a workaround for the compilation error caused by the declaration of the variable next.

ark231 avatar Jun 23 '22 15:06 ark231

I see, now it's not C99 anymore.

polluks2 avatar Jun 23 '22 18:06 polluks2

@ctrlcctrlv There is a contradiction between help message and actual behavior. Help message says the default charset is ASCII (source code), but in reality, charset is set to half-width katakana if it's not specified (source code).

ark231 avatar Jun 24 '22 02:06 ark231

@ark231 Sorry. I must have personally thought default should change as it's more realistic to the film. Nevertheless that's too big a change to spring on users, even if I'm installing a font now.

ctrlcctrlv avatar Jun 24 '22 07:06 ctrlcctrlv

I got the error @Hornobster got and also some warnings which meant functions from libintl.h was used without explicit declaration, so I modified the code like the patch below and it worked for me. fix_gettext.patch.txt content of the patch:

The latest code doesn't need this I hope because I am using a macro defined in util.h for gettext (_).

ctrlcctrlv avatar Jun 25 '22 06:06 ctrlcctrlv

I could build and install it without problems:

cmake -B build
cd build
make
sudo make install

But I found two issues that were not present before and two probably new ones.

  1. When running at console (with limited color support), it was supposed to have two shades of the selected color. e.g. a light green and a dark green. However, the light color is shown as gray for whatever -C color you choose.
  2. The rain doesn't work forever anymore. After some time (realistically, a long time) some rain columns starts to disappear, until nothing is shown. If ran with cmatrix -u 0, this problem happens after a few seconds.
  3. When using the -S argument, some rain columns starts moving right and glitching, causing some characters to appear at wrong places. Probably due to some double-width characters.
  4. A small typo during cmatrix --help, at -m: lambda mode (equivalent to -u λ), where -u was supposed to be -U.

Nevertheless, nice PR, I like that this version works properly with katakana charset. :+1:

rondao avatar Oct 10 '22 14:10 rondao

At console = Linux tty?

ctrlcctrlv avatar Oct 10 '22 18:10 ctrlcctrlv

Yes, that's what I meant, like when changing with Ctrl + Alt + F2.

Also, I tested the same thing happens with XTerm, when it's set with only 8 colors.

$ tput colors
8

Changing it to use 256 colors avoids the problem. export TERM=xterm-256color

rondao avatar Oct 10 '22 21:10 rondao

Hmm. What if you -B?

ctrlcctrlv avatar Mar 14 '23 19:03 ctrlcctrlv