Opened 7 years ago

Closed 17 months ago

#29061 closed Bug (worksforme)

manage.py makemessages throws syntax error due to incorrectly generated django.pot, again

Reported by: Kyan Owned by: Sarah Boyce
Component: Core (Management commands) Version: 2.0
Severity: Normal Keywords: gettext, makemessages, Windows
Cc: Claude Paroz Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

https://code.djangoproject.com/ticket/28773

I met the same problem, exactly like the ticket above.
I am using Django 2.0.1 and after check the django codes, I confirm that fix is in my django. But that fix doesn't work for me.
(Fix:
https://github.com/django/django/commit/4f5526e346861c0b2ffa2ea7229747c883e14432)

I changed the file in django/core/management/commands/makemessages.py

# django/core/management/commands/makemessages.py, line 193
with open(potfile, 'a', encoding='utf-8', newline='\n') as fp:

from newline="\n" to newline="\r" and it works.

Attachments (1)

201703271529397914076.png (187.6 KB ) - added by Kyan 7 years ago.
first makemessages is for "\r" version, second makemessages is for "\n" version.

Download all attachments as: .zip

Change History (10)

by Kyan, 7 years ago

Attachment: 201703271529397914076.png added

first makemessages is for "\r" version, second makemessages is for "\n" version.

comment:1 by Tim Graham, 7 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Anvansh Singh, 18 months ago

Owner: changed from nobody to Anvansh Singh
Status: newassigned

comment:3 by Anvansh Singh, 17 months ago

I am a bit unsure what would be the best place to add the regression test for it. Can really use some help with that.

comment:4 by Sarah Boyce, 17 months ago

Sorry for not looking into this in more detail. I made what I was expecting a regression test might look like https://github.com/django/django/pull/17150/ but it seems to be passing everywhere.
In the commit comment they mention that their system is Win10 with gettext 0.19.8.1. The image attached in the ticket doesn't look to be related.
I could be getting a lot of things wrong here, I may have misunderstood the ticket, or it is already fixed, or this is related to a very specific setup that is not covered by the GitHub actions, or the ticket is not valid.

I will try to get someone who is more familiar with translations to double check the ticket and point us in the right direction here 👍

comment:5 by Sarah Boyce, 17 months ago

Cc: Claude Paroz added

Claude - I hear you're the translations expert 🎉 no rush on this one, when you get some time can you check if you think this ticket is valid and how we can replicate the issue?

comment:6 by Claude Paroz, 17 months ago

I must admit I don't have any motivation to fix Windows-related issue, and even if I had, having no access to a Windows system is crippling to fix such issues. Sorry.

comment:7 by Sarah Boyce, 17 months ago

That's completely fine! I sank some time trying to install gettext on our Windows GitHub action and found the whole thing very frustrating - thank you for getting to me ⭐

comment:8 by Sarah Boyce, 17 months ago

Has patch: set
Owner: changed from Anvansh Singh to Sarah Boyce

Ok so managed to get a version of gettext installed in our GitHub action by installing poedit (via chocolatey https://community.chocolatey.org/packages/poedit) which has gettext bundled. Then I have a test as to my best guess of what the issue might be referring to and this passes.

Going to mark the ticket for a PR review but I think the PR is optional, I am very tempted to close the ticket as "worksforme" but I will let a merger decide what's most appropriate.

in reply to:  8 comment:9 by Mariusz Felisiak, 17 months ago

Has patch: unset
Resolution: worksforme
Status: assignedclosed
Triage Stage: AcceptedUnreviewed

Replying to Sarah Boyce:

Ok so managed to get a version of gettext installed in our GitHub action by installing poedit (via chocolatey https://community.chocolatey.org/packages/poedit) which has gettext bundled. Then I have a test as to my best guess of what the issue might be referring to and this passes.

Thanks for all your efforts and investigation.

Going to mark the ticket for a PR review but I think the PR is optional, I am very tempted to close the ticket as "worksforme" but I will let a merger decide what's most appropriate.

Agreed, we've not changed anything in Django itself, so it's not worth extra testing and workarounds.

Note: See TracTickets for help on using tickets.
Back to Top