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)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: adriank, Unassigned)

References

()

Details

Attachments

(2 files)

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.
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?
Blocks: 494517
No longer blocks: 494512
(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?
Attached patch patch v1Splinter Review
Attachment #379898 - Flags: review?(gandalf)
Comment on attachment 379898 [details] [diff] [review]
patch v1

r+
Attachment #379898 - Flags: review?(gandalf) → review+
that's what I'm commiting
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: