Skip to content

Conversation

@skef
Copy link
Contributor

@skef skef commented Dec 30, 2023

No description provided.

@iorsh
Copy link
Contributor

iorsh commented Dec 30, 2023

@skef, I fixed the issues in Italian translation. Please download and rebuild again, it should be better now.

@JoesCat
Copy link
Contributor

JoesCat commented Dec 30, 2023

Hi @iorsh,
if you check the CI/linux *ubuntu-latest, NoUI) test that failed, you'll note that it has 41 failures due to the updated zh_CN.po, and looking at the first two entries I suspect the following updates need similar attention.

FAILED: po/zh_CN.mo /home/runner/work/fontforge/fontforge/repo/build/po/zh_CN.mo
cd /home/runner/work/fontforge/fontforge/repo/build/po && /usr/bin/msgfmt --check -o /home/runner/work/fontforge/fontforge/repo/build/po/zh_CN.mo /home/runner/work/fontforge/fontforge/repo/po/zh_CN.po
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:994: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same

-msgid "All entries in a lookup must have the same type on line %d of %s"
-msgstr "查找中的所有条目在 %s 的第 %d 行中必须具有相同的类型"
+msgid "All entries in a lookup must have the same type on line %1$d of %2$s"
+msgstr "查找中的所有条目在 %2$s 的第 %1$d 行中必须具有相同的类型"

similar fixes would be necessary for the other languages (changing $d to %1$d and the %s to %2$s).
This file would also need fixing... ...you can find it using the command line grep, as in:
grep "All entries in a lookup must have the same type on line %" -R *

--------------------------- fontforge/featurefile.c ---------------------------
index 7ccdea7db..01cc02f65 100644
@@ -5186,7 +5186,7 @@ return;
 	else if ( lookuptype==ot_undef )
 	    lookuptype = cur;
 	else if ( lookuptype!=cur ) {
-	    LogError(_("All entries in a lookup must have the same type on line %d of %s"),
+	    LogError(_("All entries in a lookup must have the same type on line %1$d of %2$s"),
 		    tok->line[tok->inc_depth], tok->filename[tok->inc_depth] );
 	    ++tok->err_count;
     break;

Looking at the surrounding lines, looks like a tempting target to edit the other LogError lines, which also means editing the corresponding po files too. probably best as another patch...

2nd correction:

/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4266: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same

-msgid "Different fill patterns in layer %d of %s\n"
-msgstr "%s 的 %d 层中的不同填充图案\n"
+msgid "Different fill patterns in layer %1$d of %2$s\n"
+msgstr "%2$s 的 %1$d 层中的不同填充图案\n"
----------------------------- fontforge/fvfonts.c -----------------------------
index 893242c8b..f7cd991b4 100644
@@ -1509,7 +1509,7 @@ static void LayerInterpolate(Layer *to,Layer *base,Layer *other,real amount,Spli
 	    base->stroke_pen.brush.gradient!=NULL || other->stroke_pen.brush.gradient!=NULL )
 	LogError(_("I can't even imagine how to attempt to interpolate gradients in layer %d of %s\n"), lc, sc->name );
     if ( base->fill_brush.pattern!=NULL || other->fill_brush.pattern!=NULL )
-	LogError(_("Different fill patterns in layer %d of %s\n"), lc, sc->name );
+	LogError(_("Different fill patterns in layer %1$d of %2$s\n"), lc, sc->name );
     if ( base->stroke_pen.brush.pattern!=NULL || other->stroke_pen.brush.pattern!=NULL )
 	LogError(_("Different stroke patterns in layer %d of %s\n"), lc, sc->name );

...and 3rd to 41st lines would be...
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4274: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4278: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4283: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4288: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4293: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4297: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4301: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4682: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4857: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4869: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4873: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4885: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4889: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4917: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4921: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4951: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4962: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4966: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4970: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4974: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4978: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4982: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:4987: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5008: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5016: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5028: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5032: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5036: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5040: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5050: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5054: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5058: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5062: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5074: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:5269: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:6537: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:13889: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:14391: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
/home/runner/work/fontforge/fontforge/repo/po/zh_CN.po:14430: format specifications in 'msgid' and 'msgstr' for argument 2 are not the same
/usr/bin/msgfmt: found 41 fatal errors

@skef
Copy link
Contributor Author

skef commented Dec 30, 2023

I was going to work through these, and I still can, but the question is do we care more about the work of this clueless translator. One presumes that there are probably other translations with two "%s"'s (or whatever) that need reversing that we won't know about.

Should we just leave that to others/posterity or should we try to weed them out?

@iorsh
Copy link
Contributor

iorsh commented Dec 30, 2023

I was going to work through these, and I still can, but the question is do we care more about the work of this clueless translator. One presumes that there are probably other translations with two "%s"'s (or whatever) that need reversing that we won't know about.

Should we just leave that to others/posterity or should we try to weed them out?

It's a tricky question. I'd say we need to explicitly mark argument numbers in the original (en-US) messages whenever there is more than one. But this is a separate and probably huge pile of work, which also has a potential to invalidate a lot of good translations in other languages. I'd just fix these and pray that users complain when they see the mixed messages in the wild.

Alternatively, we can later contact the translator and ask him to review - the new messages are from the last year, so the memory should be fresh.

@skef
Copy link
Contributor Author

skef commented Dec 30, 2023

I'll work on fixing these in a bit, unless I hear otherwise.

@JoesCat
Copy link
Contributor

JoesCat commented Dec 30, 2023

I'll work on fixing these in a bit, unless I hear otherwise.

My recommendation is getting this group to pass first.

I looked at another po file, and think all po files here will benefit from a new pot file later, plus a merge, since the smaller files are missing a lot of msgid/msgstr - this would be a bigger task, and benefit from a push of updated po files into crowdin.

@skef
Copy link
Contributor Author

skef commented Dec 31, 2023

The existing .pot file on Crowdin seems to have strings from my plugin work, so it looks like it was updated after the last release. I'm not sure much of anything else has changed since then. I guess there would probably be a few additional strings.

And I don't think we've accepted PRs for direct changes to the .po files -- we've been redirecting those things to the crowdin site.

@JoesCat
Copy link
Contributor

JoesCat commented Dec 31, 2023

Looking at just the two files fontforge/featurefile.c and fontforge/fvfonts.c seems to suggest many lines could be updated.
The pot file would create a lot of (unnecessarily) dead lines simply adding n$ to the variables.

Would it be less work modifying the po files thru crowdin's UI, or pushing updated po files modified thru a text editor?
Guess that's a question for a later time.

@iorsh
Copy link
Contributor

iorsh commented Dec 31, 2023

Ok, so I checked how many messages have two or more unnumbered arguments:
cmake -GNinja ..; ninja potfiles
cat po/FontForge.pot | grep "%[^0-9].*%[^0-9]" | wc -l
There are at least 389, as I may have missed some multiline messages.

For me this basically means that we will have to leave the source as is and just ask the translator to review his contributions. Besides, gettext utils don't seem to have any method to enforce argument numbering, so this is probably not a good idea anyway.

@skef
Copy link
Contributor Author

skef commented Dec 31, 2023

The only tractable way to approach the problem would be to limit the question to the strings this translator translated, which were all probably since the last release (he was working about 7 months ago). I know how to tell who entered an individual translation but not how to get their full list for the project.

I agree we can just not worry about this. I'm not sure someone will complain if they see bad translations but that's just where we are.

(Annoying that CrowdIn itself doesn't have more systematic checking for C format translations to give earlier feedback ...)

I'm gonna take care of these tonight, probably by editing the file and doing an upload of just the changed translations

@iorsh
Copy link
Contributor

iorsh commented Dec 31, 2023

@skef, it looks like we are clashing on the CrowdIn fixes right now. I've done till the line zh_CN.po:5036 (30 errors)

@skef
Copy link
Contributor Author

skef commented Dec 31, 2023

I just changed all of them via upload

@skef
Copy link
Contributor Author

skef commented Dec 31, 2023

I resynced after the changes and FWIW it built on my Arch Linux setup, which it hasn't for a long while.

@skef
Copy link
Contributor Author

skef commented Dec 31, 2023

OK, we're past that but the _pyhook tests are failing on windows. This is the test of the windows loadable module, which it apparently can't find.

@iorsh
Copy link
Contributor

iorsh commented Dec 31, 2023

The MacOS failure is due to intermixed Python versions (it has 3.10, 3.11.6, 3.11.7, 3.12 and maybe more) installed on the GitHub runner. I'm checking now how to align them properly

@skef
Copy link
Contributor Author

skef commented Dec 31, 2023

Normal MacOS tests pass but then the smoketest fails because it can't load psMat. But test1003 imports psMat. Some weird path problem?

@iorsh
Copy link
Contributor

iorsh commented Dec 31, 2023

Normal MacOS tests pass but then the smoketest fails because it can't load psMat. But test1003 imports psMat. Some weird path problem?

Yes. test1003 uses the python executable that was discovered by CMake, but the smoketest is called right from main.yml using the default python3 executable found on the PATH

@skef
Copy link
Contributor Author

skef commented Dec 31, 2023

@iorsh The messages at the start of the test indicate PYTHON is set to "python3" also, and those tests succeed. However, the pyhook shim systestdriver.c sets PYTHONPATH to the passed libdir, which is presumably based on the below:

MacOS module installation locations are

-- Installing: /Users/runner/work/fontforge/fontforge/target/lib/python3.12/site-packages/fontforge.so
1364
-- Installing: /Users/runner/work/fontforge/fontforge/target/lib/python3.12/site-packages/psMat.so

So one missing piece of info: why expect that directory to ever be in the (explicit or "effective") PYTHONPATH?

@skef
Copy link
Contributor Author

skef commented Dec 31, 2023

Damn, logs from the last successful PR build in February are expired so we can't compare ...

@skef
Copy link
Contributor Author

skef commented Dec 31, 2023

Ok, python 3.11.6_1 seems to be a MacOS dependency, so we may just have clashing pythons.

@iorsh
Copy link
Contributor

iorsh commented Dec 31, 2023

Test1003 and other pyhook tests ignore $PYTHON and use CMake ${Python3_EXECUTABLE} instead - https://github.com/fontforge/fontforge/blob/master/cmake/TestUtils.cmake#L78

@skef
Copy link
Contributor Author

skef commented Dec 31, 2023

Hmm. We may have a more basic problem here. Our build is Homebrew based. I suspect that as far as Python 3 goes most users are migrating to the (more) official builds on python.org: https://www.python.org/downloads/macos/

Unfortunately the build needs a lot of UI stuff installed through homebrew.

Ideally we would build the MacOS app the way we do now (which seems to work) but then build the (UI-less) module to work with whatever version of Python 3 is installed by default. That means the latter can't be "canned" for the installer, but to be honest I don't know what we do about that as it is.

@skef
Copy link
Contributor Author

skef commented Dec 31, 2023

Well, all the builds got past the l10n issues so I think I'm going to merge this. I doubt I'll dig into the remaining problems anytime in the next couple of days (and maybe not then) but I encourage others to do so. Open test PRs as you see fit.

@skef skef merged commit 642d8a3 into fontforge:master Dec 31, 2023
@iorsh
Copy link
Contributor

iorsh commented Dec 31, 2023

Closes #5251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants