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.

Sign up for GitHub

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

Jump to bottom

Crowdin: represent whitespace in code instead of in string #4557

Merged
merged 4 commits into from May 5, 2021
Merged

Crowdin: represent whitespace in code instead of in string #4557

merged 4 commits into from May 5, 2021

Conversation

luzpaz
Copy link
Contributor

@luzpaz luzpaz commented Mar 2, 2021

Removing possible whitespace from strings lead to more accurate translations

@luzpaz
Copy link
Contributor Author

luzpaz commented Mar 2, 2021

@paullee0 do you mind taking a look at https://github.com/FreeCAD/FreeCAD/pull/4557/files#diff-3ec595fa09c804aea2062712c218dd2a145fb936b846a9671b67f244d96db9f4 ?

@luzpaz luzpaz force-pushed the crowdin/whitespaces-substituted-into-code branch from d196d4f to d6ec0c8 Compare March 2, 2021 16:13
@paullee0
Copy link
Contributor

paullee0 commented Mar 2, 2021

@paullee0 do you mind taking a look at https://github.com/FreeCAD/FreeCAD/pull/4557/files#diff-3ec595fa09c804aea2062712c218dd2a145fb936b846a9671b67f244d96db9f4 ?

OK, you mean the commit to delete a few whitespace in ArchStairs.py right?
Though not sure why there had been whitespace, and why it is not desirable thus removed, do not see any problem doing so :)
Anything you expect?

@luzpaz luzpaz force-pushed the crowdin/whitespaces-substituted-into-code branch 4 times, most recently from 16fcd39 to 4c0c471 Compare March 3, 2021 19:00
@luzpaz luzpaz added the Translation Translation and localization related label Mar 4, 2021
@luzpaz
Copy link
Contributor Author

luzpaz commented Mar 4, 2021

@yorikvanhavre when review is complete, will this go in to the 0.19 or 0.20 ?

@luzpaz
Copy link
Contributor Author

luzpaz commented Mar 9, 2021

@chennes can you review some of these. I know some of them are wrong.

@luzpaz luzpaz force-pushed the crowdin/whitespaces-substituted-into-code branch from 4c0c471 to 0c99c23 Compare March 9, 2021 16:17
@luzpaz
Copy link
Contributor Author

luzpaz commented Mar 10, 2021

@yorikvanhavre can you review this and after merge push to crowdin (since there are a lot of errors fixed + translations now available in the StartWB (see #4606 )

@luzpaz luzpaz mentioned this pull request Mar 10, 2021
Merged
3 tasks
@luzpaz luzpaz force-pushed the crowdin/whitespaces-substituted-into-code branch 2 times, most recently from a79926e to fedb8d3 Compare March 10, 2021 19:07
@luzpaz
Copy link
Contributor Author

luzpaz commented Mar 11, 2021

Travis is complaining about https://travis-ci.org/github/FreeCAD/FreeCAD/jobs/762361197#L7550-L7563
fedb8d3#diff-8883e954b4828659c14024871fb8a5af47ca0149506decf625c5e0fdc92491b2
Any insight?

@luzpaz luzpaz force-pushed the crowdin/whitespaces-substituted-into-code branch from fedb8d3 to b5fcbaf Compare March 11, 2021 03:44
@luzpaz
Copy link
Contributor Author

luzpaz commented Mar 11, 2021

Ok, fixes pass CI. Please do final review

@luzpaz luzpaz marked this pull request as ready for review March 11, 2021 14:26
@luzpaz
Copy link
Contributor Author

luzpaz commented Mar 13, 2021

@wwmayer how would you prefer I squash this PR ?

@yorikvanhavre
Copy link
Member

I would prefer to merge these things after 0.19 is fully out, in case we need to pull translations once more. If this PR is in (and the strings have been sent to crowdin), the translations we get from crowdin will differ from the 0.19 sources...

@wwmayer
Copy link
Contributor

wwmayer commented Mar 28, 2021

@yorikvanhavre feel free to merge it when you think the time is right

yorikvanhavre
yorikvanhavre reviewed Mar 30, 2021
View reviewed changes
src/Mod/Draft/importDXF.py
@@ -3860,7 +3860,7 @@ def export(objectslist, filename, nospline=False, lwPoly=False):
filename = filename.encode("utf8")
dxf.saveas(filename)

FCC.PrintMessage("successfully exported " + filename + "\n")
FCC.PrintMessage("successfully exported" + " " + filename + "\n")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to add a translate() here too I guess

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yorikvanhavre there are no other translate()'s in the PrintMessage() in the rest of this file so maybe that would be a separate PR?

yorikvanhavre
yorikvanhavre reviewed Mar 30, 2021
View reviewed changes
src/Mod/OpenSCAD/exportCSG.py
@@ -269,4 +269,4 @@ def export(exportList,filename):
csg.write("}\n}\n")
# close file
csg.close()
FreeCAD.Console.PrintMessage("successfully exported "+filename)
FreeCAD.Console.PrintMessage("successfully exported" + " " + filename)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first part should go into a translate()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yorikvanhavre there are no other translate()'s in the PrintMessage() in the rest of this file so maybe that would be a separate PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure!

yorikvanhavre
yorikvanhavre reviewed Mar 30, 2021
View reviewed changes
src/Mod/Part/Gui/TaskCheckGeometry.cpp
@@ -149,7 +149,7 @@ QString checkStatusToString(const int &index)
}
if (index > 33 || index < 0)
{
QString message(QObject::tr("Out Of Enum Range: "));
QString message(QObject::tr("Out Of Enum Range:") + QString::fromLatin1(" "));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO if you touch not only string contents but also code itself, you should not skip CI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough

yorikvanhavre
yorikvanhavre reviewed Mar 30, 2021
View reviewed changes
src/Mod/Start/Gui/Resources/translations/StartPage.ts
<location filename="../../DlgStartPreferences.ui" line="289"/>
<source>If this is checked, if a style sheet is specified in General preferences, it will be used and override the colors below</source>
<translation type="unfinished"></translation>
</message>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is a ghost translation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mention it in the commit message:

For some reason updatets.py is not successfully removing obsolete translations. This is a long standing issue. The workaround is to manually remove them from the .ts file.

ref: https://forum.freecadweb.org/viewtopic.php?f=21&t=51825&p=483474#p482519
crowdin: https://crowdin.com/translate/freecad/7254/en-pl?filter=basic&value=0#6572886

@luzpaz
Copy link
Contributor Author

luzpaz commented Apr 18, 2021

If there are no other comments, this PR should be ready to go

@luzpaz
Crowdin: represent whitespace in code instead of in string [skip ci]
f2b93a7 
Removing possible whitespace from strings lead to more accurate translations
@luzpaz
Crowdin: remove obsolete translation manually [skip ci]
3b9882a 
For some reason updatets.py is not successfully removing obsolete translations. This is a long standing issue. The workaround is to manually remove them from the .ts file.
@luzpaz
Crowdin: remove obsolete translation manually [skip ci]
5cef2ce 
For some reason updatets.py is not successfully removing obsolete translations. This is a long standing issue. The workaround is to manually remove them from the .ts file.

ref: https://forum.freecadweb.org/viewtopic.php?f=21&t=51825&p=483474#p482519  
crowdin: https://crowdin.com/translate/freecad/7254/en-pl?filter=basic&value=0#6572886
@luzpaz
Crowdin: represent whitespace in code instead of string
1388db3
@luzpaz luzpaz force-pushed the crowdin/whitespaces-substituted-into-code branch from b5fcbaf to 1388db3 Compare April 19, 2021 10:27
@luzpaz luzpaz changed the title Crowdin: represent whitespace in code instead of in string [skip ci] Crowdin: represent whitespace in code instead of in string Apr 19, 2021
@yorikvanhavre yorikvanhavre merged commit 131cdd1 into FreeCAD:master May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chennes chennes chennes left review comments

@yorikvanhavre yorikvanhavre yorikvanhavre approved these changes

Assignees
No one assigned
Labels
Translation Translation and localization related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@luzpaz @paullee0 @yorikvanhavre @wwmayer @chennes