Opened 16 years ago

Last modified 13 years ago

#7704 closed Bug

JS comments put after statements break make-messages.py output — at Version 10

Reported by: Robby Dermody Owned by: Ned Batchelder
Component: Internationalization Version: 1.0
Severity: Normal Keywords: djangojs, make-messages
Cc: robbyd@…, ned@…, ionel.mc@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description (last modified by Łukasz Rekucki)

To test, make a JS file (say myfile.js) with the following valid JS content:

var a = 1;
if(a != 2 && a != 5) //this comment breaks the file
{
    //this does not
    alert(gettext("foobar"));
}

Running make-messages.py -d djangojs -a will then yield the following output for that (in the myfile.js.py intermediate file it produces):

var a = 1;
if(a != 2 && a != 5) //this comment breaks the file
{
#this does not
    alert(gettext("foobar"));
}

As you can see, the comment after the if statement was not replaced, and since xgettext is then run in Perl mode, it seems to choke on that input. The result depends on the exact code: This example will cause only that next gettext("foobar") not to be generated (ones further down in the code will). With other code I had that had a similar line, nothing was generated. The failure is silent and the only way to know is by checking the gettext output (or lack thereof :).

This is due to the regexp in make-messages: pythonize_re = re.compile(r'\n\s*//') and then the replacement code: src = pythonize_re.sub('\n#', src)

That assumes that comments come after newlines. I'm not submitting a patch right now because I'm unsure about the best regexp to use for this that will get all the valid JS comment cases (or if that is even something the django devs want to do). At the very least, if you all choose not to address this in the code, there should be a note in the documentation telling folks to always put JS comments on their own lines.

As make-messages.py is now included in django-admin AFAIK, I've categorized it to that.

Change History (11)

comment:1 by Robby Dermody, 16 years ago

Cc: robbyd@… added

comment:2 by anonymous, 16 years ago

I also faced this *bug*.

It also seems that class comments like this one :

/**
 * ***************************** 
 * AddModule main / window
 * @constructor
 * @class MyDesktop.AddModule
 * *****************************
 */ 

breaks the following gettext references. If I remove the "/" in " * AddModule main / window" .. it works.

comment:3 by Sung-jin Hong, 16 years ago

Triage Stage: UnreviewedAccepted

Seems to me like a correct and reproducible bug.

comment:4 by anonymous, 16 years ago

Version: SVN1.0

comment:5 by anonymous, 16 years ago

This feature is completely broken at the current state.

comment:6 by anonymous, 16 years ago

I just quite some time on trying to find out why some strings from my JS files weren't translated.
I was aware of the end of line comment probleme and some other as I post the 07/11/08 08:52:41 comment.

Any way something else was breaking the process.
I figured out that this line

,this.split = elem.split ? true : false

among others causes problem.

Anyway. I asked myself why only "comment out" the JS comments before processing the fake "js.py" file with gettext ?
Seems to me that we can "comment out" all the lines that don't have gettext or ngettext in it.

pythonize_re2 = re.compile('gettext')
src = open(os.path.join(dirpath, file), "r").readlines()
dest = open(os.path.join(dirpath, '%s.py' % file), "wb")
for line in src :
 if not pythonize_re2.search(line):
  dest.write('#%s' % line)
 else:
  dest.write(line)
dest.close()

I went line by line due to my regex bad competences. a bit slower.
I made a quick

grep -r 'gettext' /js/templates | grep '?'

then I change some lines

return values.value ? gettext("Oui") : gettext("Non");

to 

return values.value ? 
 gettext("Oui") : gettext("Non");  

I looked at the latest translation of all JS files to see if 1- it was present (not the case before) or 2- line number was correct.
All files were well parse

Not a solution a quick hack.

xav

comment:7 by Peter Baumgartner, 14 years ago

Severity: Normal
Type: Bug

comment:8 by Ned Batchelder, 14 years ago

Owner: changed from nobody to Ned Batchelder
Status: newassigned

comment:9 by Ned Batchelder, 14 years ago

(edits here incorporated into main description)

Last edited 14 years ago by Ned Batchelder (previous) (diff)

comment:10 by Łukasz Rekucki, 14 years ago

Description: modified (diff)

Fixed formating in description :)

by Ned Batchelder, 14 years ago

Attachment: makemessages.diff added

Adapt makemessages to use JsLexer

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