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

wxGUI: added install notranslation function #156

Merged
merged 2 commits into from
Nov 5, 2019
Merged

wxGUI: added install notranslation function #156

merged 2 commits into from
Nov 5, 2019

Conversation

marisn
Copy link
Contributor

@marisn marisn commented Oct 16, 2019 •
edited by neteler
Loading

_ is needed for startup code itself. In case of error, provide no translation but allow to continue,
fixes this error:

NameError: global name '_' is not defined

Fixes trac 3875 (https://trac.osgeo.org/grass/ticket/3875)

wenzeslaus
wenzeslaus requested changes Oct 22, 2019
View reviewed changes
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

Answering in the scope of these changes: Please, make the buildins fix into a function to 1) not repeat the code 2) to make it explicit and documented.

@wenzeslaus
Copy link
Member

Thank you marisn. This PR accounts for the no-translation case in a better way than the top-level gettext.install call removed in 2246f5b. I think we should (squashe and) merge it after adding the function there.

I need to mention broader context which is my reservations about having _ in the builtins in the first place (https://trac.osgeo.org/grass/ticket/3790). Nevertheless, this code actually seems better even in case we change the direction and not have _ in builtins.

What I don't know is how gettext.install behaves when called in the no-translation case. Apparently, it worked just fine before, so perhaps there is some potential for simplification.

marisn and others added 2 commits October 30, 2019 12:31
@marisn
Python3 migration: Always provide encoding for decode as the rest of …
7797c8a 
…code assumes str instead of bytes.

Fixes C and POSIX case of OSGeo#3922 and OSGeo#3875
@marisn
wxGUI: _ is needed for startup code itself. In case of error, provide…
a8516d5 
… no translation but allow to continue.

Fixes trac OSGeo#3875
@marisn
Copy link
Contributor Author

marisn commented Oct 30, 2019

Had to fix C and POSIX corner case too.

@neteler
Copy link
Member

neteler commented Oct 31, 2019

Is this relevant for the upcoming 7.8.1RC2?

@marisn
Copy link
Contributor Author

marisn commented Nov 1, 2019

Is this relevant for the upcoming 7.8.1RC2?

As it fixes starting with C locale or when there is some problem with installed locales, yes. Ditto for finishing transition to Python 3.

@neteler
Copy link
Member

neteler commented Nov 3, 2019

@wenzeslaus anything missing here? can it be merged and backported?

wenzeslaus
wenzeslaus approved these changes Nov 4, 2019
View reviewed changes
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

The code is good. I suggest improve the commit messages. If the two commits should be indeed together, then I suggest squashing and improving the commit message, esp. describing sufficiently the motivation. The links to Trac issues should be now done as URLs for clarity, not as #0000.

@neteler neteler changed the title wxGUI: _ is needed for startup code itself. In case of error, provide… wxGUI: added install notranslation function Nov 4, 2019
neteler
neteler reviewed Nov 4, 2019
View reviewed changes
lib/init/grass.py
def install_notranslation():
# If locale is not supported, _ function might be missing
# This function just installs _ as a pass-through function
# See trac #3875 for details
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# See trac #3875 for details
# See trac https://trac.osgeo.org/grass/ticket/3875 for details

Copy link
Member

Choose a reason for hiding this comment

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

@marisn pls apply this change, I have no permission to do so in your branch

@neteler
Copy link
Member

neteler commented Nov 5, 2019

ok, to get it into 7.8.1RC2 I'll just merge it now

@neteler neteler merged commit 63d0825 into OSGeo:master Nov 5, 2019
neteler pushed a commit that referenced this pull request Nov 5, 2019
@marisn @neteler
wxGUI: added install no translation function (#156)
25835a2 
- wxGUI: _ is needed for startup code itself. In case of error, provide no translation but allow to continue, fixing this error: `NameError: global name '_' is not defined`
- Python3 migration: Always provide encoding for decode as the rest of code assumes str instead of bytes.
- Fixes C and POSIX case of https://trac.osgeo.org/grass/ticket/3922 and https://trac.osgeo.org/grass/ticket/3875
@neteler
Copy link
Member

neteler commented Nov 5, 2019

Backported to relbranch78 in 25835a2

@marisn marisn deleted the issue_3875 branch November 10, 2019 09:27
petrasovaa pushed a commit to petrasovaa/grass that referenced this pull request Dec 3, 2019
@marisn @petrasovaa
wxGUI: added install no translation function (OSGeo#156)
606b9e6 
- wxGUI: _ is needed for startup code itself. In case of error, provide no translation but allow to continue, fixing this error: `NameError: global name '_' is not defined`
- Python3 migration: Always provide encoding for decode as the rest of code assumes str instead of bytes.
- Fixes C and POSIX case of https://trac.osgeo.org/grass/ticket/3922 and https://trac.osgeo.org/grass/ticket/3875
landam pushed a commit to landam/grass that referenced this pull request Jan 28, 2020
@marisn @landam
wxGUI: added install no translation function (OSGeo#156)
8e66cf2 
- wxGUI: _ is needed for startup code itself. In case of error, provide no translation but allow to continue, fixing this error: `NameError: global name '_' is not defined`
- Python3 migration: Always provide encoding for decode as the rest of code assumes str instead of bytes.
- Fixes C and POSIX case of https://trac.osgeo.org/grass/ticket/3922 and https://trac.osgeo.org/grass/ticket/3875
@marisn marisn mentioned this pull request Feb 21, 2020
Closed
@neteler neteler added this to the 7.8.1 milestone Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neteler neteler neteler left review comments

@wenzeslaus wenzeslaus wenzeslaus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
7.8.1
Development

Successfully merging this pull request may close these issues.
3 participants
@marisn @wenzeslaus @neteler