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

Draft: Fix several translation issues #5445

Merged
merged 4 commits into from Jan 28, 2022

Conversation

Roy-043
Copy link
Contributor

@Roy-043 Roy-043 commented Jan 26, 2022

PR to fix several translation issues. See forum topic.

Forum topic:
https://forum.freecadweb.org/viewtopic.php?f=23&t=65346

Additionally:

  • Removed the faulty _tr and _qtr functions from translate.py. They are no longer used by the Draft or Arch WB.
  • In view_base.py: Fixed wrong use of variables in QT_TRANSLATE_NOOP calls and changed the tooltips from 'hatch' pattern to 'SVG' pattern.
  • Your pull request is confined strictly to a single module. That is, all the files changed by your pull request are either in App, Base, Gui or one of the Mod subfolders. If you need to make changes in several locations, make several pull requests and wait for the first one to be merged before submitting the next ones
  • In case your pull request does more than just fixing small bugs, make sure you discussed your ideas with other developers on the FreeCAD forum
  • x] Your branch is rebased on latest master git pull --rebase upstream master
  • All FreeCAD unit tests are confirmed to pass by running ./bin/FreeCAD --run-test 0
  • All commit messages are well-written ex: Fixes typo in Draft Move command text
  • Your pull request is well written and has a good description, and its title starts with the module name, ex: Draft: Fixed typos
  • Commit messages include issue #<id> or fixes #<id> where <id> is the FreeCAD bug tracker issue number in case a particular commit solves or is related to an existing issue on the tracker. Ex: Draft: fix typos - fixes #0004805

@github-actions github-actions bot added the WB Draft Related to the Draft Workbench label Jan 26, 2022
'MenuText': QT_TRANSLATE_NOOP("Draft_BezCurve", "Bezier curve"),
'ToolTip': QT_TRANSLATE_NOOP("Draft_BezCurve", "Creates an N-degree Bezier curve. The more points you pick, the higher the degree.\nCTRL to snap, SHIFT to constrain.")}
'Accel': 'B, Z',
'MenuText': QT_TRANSLATE_NOOP('Draft_BezCurve', 'Bézier curve'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are the " changed to '? Standard should be " as in the rest of the file and PEP8

Copy link
Member

Choose a reason for hiding this comment

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

@Roy-043 this appears to have happened at a few places in this changeset -- can you revise to use double quotes?

Copy link
Contributor Author

@Roy-043 Roy-043 Jan 28, 2022

Choose a reason for hiding this comment

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

If you look carefully you will see that in the original file a mix on single and double quotes are used even while defining the same dictionary. So there is no 'standard' in the file. Also: the original file was recently refactored.

But I'll fix this anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, actually there was no need to fix this:
https://forum.freecadweb.org/viewtopic.php?f=8&t=65753

@benj5378 Can you confirm please.

Copy link
Contributor

@benj5378 benj5378 Jan 28, 2022

Choose a reason for hiding this comment

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

@Roy-043 I don't see any specific conclusion in the forum thread? They mention some personal exceptions for when they wouldn't use PEP8.

Regarding Zolko's comment, I agree with him, but it's for a whole other case: naming of variables and functions. Not code formatting regarding the use of symbols, spaces, line breaks etc.

I noticed that the file is mixed, but when changes occur on, I'd personally prefer it was always changed towards PEP8.

Conclusion, do whatever you want and I'll make a seperate PR afterwards doing PEP8

Copy link
Contributor Author

@Roy-043 Roy-043 Jan 28, 2022

Choose a reason for hiding this comment

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

PEP8 allows both single and double quotes.
Link provided in the 1st forum topic answer:
https://www.python.org/dev/peps/pep-0008/#string-quotes

So your initial comment here does not make sense.

Standard should be " as in the rest of the file

I noticed that the file is mixed

And with that you initial comment makes even less sense.

Conclusion:
Effectively there is no standard governing this

Striving for consistent use of double or single quotes in the Python code of FreeCAD is a lost cause if ever I saw one. But, enough said, let's merge!

Copy link
Member

Choose a reason for hiding this comment

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

As you say 😎

@freecadci
Copy link

pipeline status for feature branch PR_5445. Pipeline 457078691 was triggered at a70df35. All CI branches and pipelines.

@freecadci
Copy link

pipeline status for feature branch PR_5445. Pipeline 458496873 was triggered at 58360bd. All CI branches and pipelines.

@chennes chennes merged commit 45ef3ed into FreeCAD:master Jan 28, 2022
@Roy-043 Roy-043 deleted the Draft-Fix-several-translation-issues branch January 29, 2022 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WB Draft Related to the Draft Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants