Skip to content

gh-130195: Remove unimplemented option from pygettext #130196

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

Merged
merged 7 commits into from
Feb 18, 2025

Conversation

StanFromIreland
Copy link
Contributor

@StanFromIreland StanFromIreland commented Feb 16, 2025

@StanFromIreland
Copy link
Contributor Author

Requesting @gvanrossum

Co-authored-by: Tomas R. <[email protected]>
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

With those two changes I am fine with this (there's a FIXME comment and you never know when someone decides to volunteer to do so :-).

@gvanrossum
Copy link
Member

@warsaw Are we missing anything here? Maybe we should just remove the -a/--extract-all option? Maybe it's already implemented? What is it even supposed to do? Why is it there? Am I asking too many questions?

@StanFromIreland
Copy link
Contributor Author

Questions are always good ;-)

@tomasr8
Copy link
Member

tomasr8 commented Feb 17, 2025

@gvanrossum To answer your questions:

Maybe it's already implemented?

It's not, it's currently a noop

What is it even supposed to do?

AFAICT, it was supposed to be the equivalent of the --extract-all option from xgettext. When enabled, it extracts all strings from a Python source file, so for example these are all extracted:

x = 'foo'
y = {'bar': 'baz'}

I suggested to deprecate and eventually remove this option because I don't see much use for this option in the context of a gettext extraction tool. You typically only ever want to extract strings which are marked for extraction (e.g. via _('foo')). There could be a case where you want to extract docstrings so that you can have multilingual documentation, but pygettext already has an option for that (--docstrings).

Yes, we could always implement it, but personally I don't think it'd be that useful so I think removing it makes more sense.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Yes!

@gvanrossum gvanrossum merged commit 4374e1d into python:main Feb 18, 2025
39 checks passed
@gvanrossum
Copy link
Member

I don’t think we should backport this, we’d be adding a deprecation retroactively.

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot aarch64 RHEL8 LTO 3.x has failed when building commit 4374e1d.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/338/builds/8097) and take a look at the build logs.
  4. Check if the failure is related to this commit (4374e1d) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/338/builds/8097

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-aarch64.lto/build/Lib/threading.py", line 1054, in _bootstrap_inner
    self.run()
    ~~~~~~~~^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-aarch64.lto/build/Lib/threading.py", line 996, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-aarch64.lto/build/Lib/test/test_interpreters/test_stress.py", line 30, in task
    interp = interpreters.create()
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-aarch64.lto/build/Lib/test/support/interpreters/__init__.py", line 76, in create
    id = _interpreters.create(reqrefs=True)
interpreters.InterpreterError: interpreter creation failed
k

@StanFromIreland StanFromIreland deleted the remove-a branch February 18, 2025 08:06
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.

4 participants