Closed Bug 409956 Opened 12 years ago Closed 12 years ago

Python preprocessor ignores #filter emptyLines

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kairo, Assigned: Pike)

Details

(Keywords: fixed1.9.0.2)

Attachments

(1 file)

Bug 409805 comment #2 talks of a case that would have worked with the perl preprocessor but doesn't work with the python version (AFAIK I had tried both versions when working on bug 397246):

|#filter emptyLines| should remove all the empty lines until |#unfilter emptyLines| but the python preprocessor seems to ignore that.
It's implemented inhttp://mxr.mozilla.org/mozilla/source/config/Preprocessor.py#376.

I wonder if there's a line endings problem in that defines.inc.
We had that with multiple incarnations of that file, and it's always unix line endings from what I can tell...
I found the problem, and I have a patch and a test case offline. I'll attach that next year.
Assignee: nobody → l10n
It's been "next year" now for some time, did this patch get lost? I saw today that we are running into the same problem with normal SeaMonkey langpacks :(
So the basic culprit was that the do_filter routine returned an empty string, and write appended a line ending.

Now I do a replace, so empty stays empty.

Going through this, I moved over to open with 'rU' for universal line ending support, which reads in files correctly independent of OS line endings, and converts them to '\n', which makes the checks easier. That only works for real files, though, it seems.

Added a few tests to verify the behaviour here, too.
Attachment #322570 - Flags: review?(ted.mielczarek)
Attachment #322570 - Flags: review?(ted.mielczarek) → review+
Marking FIXED, http://hg.mozilla.org//mozilla-central/index.cgi/rev/4157b3484b8e.

Robert, you want this on CVS trunk, right?
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: wanted1.9.0.x?
Resolution: --- → FIXED
Yes, please, at least for the time being where SeaMonkey (and others) are all still based on CVS.
Comment on attachment 322570 [details] [diff] [review]
use universal line ending io and replace lineending, don't add

This stuck on mozilla-central, I'd like to land it on cvs trunk, too.
Attachment #322570 - Flags: approval1.9.0.1?
Attachment #322570 - Flags: approval1.9.0.1? → approval1.9.0.2?
Robert, since SeaMonkey is moving to hg, is this still wanted on the 1.9 branch?
After the switch, I don't care if it's on 1.9.0, but not sure if Axel would like to have it for e.g. easing Firefox localizers' work.
Yes, we should take this for fx3.0.x localizations. The line ending stuff discovered other bugs.
Actually, drop the rationale.

The rationale is really that I hope to land other patches to config which may very well touch Preprocessor.py, and it'd be good to have that tool on par with mozilla-central to reduce the risk in merges.
Comment on attachment 322570 [details] [diff] [review]
use universal line ending io and replace lineending, don't add

Approved for 1.9.0.2. Please land in CVS. a=ss
Attachment #322570 - Flags: approval1.9.0.2? → approval1.9.0.2+
Landed in CVS, too.
Keywords: fixed1.9.0.2
Flags: wanted1.9.0.x? → wanted1.9.0.x+
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.