Closed
Bug 495051
Opened 15 years ago
Closed 15 years ago
[silme] FileFormatClient._should_ignore broken if "ignore" is a function
Categories
(Mozilla Localizations :: Infrastructure, defect)
Mozilla Localizations
Infrastructure
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: adriank, Unassigned)
References
()
Details
Attachments
(2 files)
920 bytes,
patch
|
zbraniecki
:
review+
|
Details | Diff | Splinter Review |
1.75 KB,
patch
|
Details | Diff | Splinter Review |
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/4aa68586ff68/lib/silme/io/clients.py#l286 There is no such element as "query", so we are getting an exception here. Changing that line to: return ignore(path, elems) resolves the problem.
Comment 1•15 years ago
|
||
I'm wondering if it makes sense to give path and elems to the callback function, or should we give it a full relative path. Topic for discussion: I think the way it works is a little confusing. We're offering a *list* of options and if any of them is on the ignore list we return True. Otherwise False. It's useful for how we use it today, but may not be perfect for the future. Should we fire one _should_ignore per element?
Reporter | ||
Updated•15 years ago
|
Reporter | ||
Comment 2•15 years ago
|
||
(In reply to comment #1) > I'm wondering if it makes sense to give path and elems to the callback > function, or should we give it a full relative path. I'm starting to use _should_ignore for JARs in compare-locales2: I need there two values: the path to the now_to_be_read-file, and the path submitted to silme.io.jar.get_l10npackage, which is the internal-path for files I want to read (note: to achieve that, I'm reading the JAR-file before firing get_l10npackage). And if the path to the now_to_be_read-file doesn't start with the submitted path, I'm ignoring it. That's meant to be the replacement for this: http://hg.mozilla.org/users/akalla_aviary.pl/silme-patched/file/0fefc2f5a4f5/lib/mozilla/io/jar.py#l33 Unfortunatelly, we are currently submitting just the "relpath", and not the full path in sile.io.jar: http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/4aa68586ff68/lib/silme/io/jar.py#l66 , which is not enough to be able to limit what we are reading from a JAR-file... > Topic for discussion: > I think the way it works is a little confusing. > > We're offering a *list* of options and if any of them is on the ignore list we > return True. Otherwise False. > > It's useful for how we use it today, but may not be perfect for the future. > Should we fire one _should_ignore per element? I don't see a difference to the current behavior - we'd still need to make a decision at the end: True or False, wouldn't we?
Reporter | ||
Comment 3•15 years ago
|
||
Attachment #379898 -
Flags: review?(gandalf)
Comment 4•15 years ago
|
||
Comment on attachment 379898 [details] [diff] [review] patch v1 r+
Attachment #379898 -
Flags: review?(gandalf) → review+
Comment 5•15 years ago
|
||
that's what I'm commiting
Comment 6•15 years ago
|
||
branch: http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/rev/b44c4febc1e7 trunk: http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/rev/b0c9658a9e47
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•