Version 3.0 #137
Version 3.0 #137
Conversation
|
Related to #136 |
|
@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 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 It's certainly safe. |
|
Thank you for this pull request. This is indeed a massive change. I will look at it and get back soon. |
|
Wow, @ctrlcctrlv , that looks great. Using the first method with autoreconf - configure does not get very far, configure.ac: warning: AM_GNU_GETTEXT is used, but not AM_GNU_GETTEXT_VERSION or AM_GNU_GETTEXT_REQUIRE_VERSION I run macos Big Sur (11.6) and autoreconf version 2.71 installed via homebrew. The second method with cmake is installed (version 3.21.4) via homebrew, automake is version 1.16.5, also installed via homebrew. |
|
I only tested building with So do e.g. |
|
@abishekvashok yes indeed, I'm sorry, for some context as to how this happened basically I used |
|
Thanks, @ctrlcctrlv. |
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. |
no translations yet anyway
|
@Nithilher I disabled libintl by default as we don't have any translations yet anyway 😅 |
|
-c causes black characters on macOS. Also black background on the characters. The v2.0 release shows transparent background on the characters. |
|
|
abishekvashok
added
the
Changes requested
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
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. |
|
By the way I added flag |
abishekvashok
added
Testing
|
@abishekvashok Any news on this getting merged and released ? |
|
Hi, I'm trying to build with the Makefile generated with Am I missing some dependency? |
|
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 |
|
The default supplied cmake config does indeed fail with the message @Hornobster is getting, I had to run it as |
|
I got the error @Hornobster got and also some warnings which meant functions from 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 thanks for this diff, I will apply it to this PR test, and get back :) |
|
@ark231 Why the block in case 'u'? |
|
@polluks2 |
|
I see, now it's not C99 anymore. |
|
@ctrlcctrlv There is a contradiction between help message and actual behavior. |
|
@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. |
The latest code doesn't need this I hope because I am using a macro defined in util.h for gettext ( |
|
I could build and install it without problems: But I found two issues that were not present before and two probably new ones.
Nevertheless, nice PR, I like that this version works properly with katakana charset. 👍 |
|
At console = Linux tty? |
|
Yes, that's what I meant, like when changing with Also, I tested the same thing happens with XTerm, when it's set with only 8 colors. Changing it to use 256 colors avoids the problem. |
Without this, 32nd of the charset will be, whatever it is, treated as ' ', which obviously isn't what is intended.
|
Hmm. What if you |
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:
cmatrix-iwhich inverts colors of headingsI once again apologize for the absurd size of this PR.