search

rubenv / node-apk-parser

Extract Android Manifest info from an APK file.
MIT License
79 stars 25 forks source link

Windows fix & update #2

Open jdarling opened 10 years ago

jdarling commented 10 years ago

Fixed so it works on windows, switched to adm-zip for decompression, and added progress for progress monitoring when downloading Build Tools. Also fixed a minor bug in the parser where it wasn't looking for C: symbols and skipping them. Really it should just skip any unknown symbols.

rubenv commented 10 years ago

Cool! Do you have an example of such a missing string that we can add as a test case?

jdarling commented 10 years ago

Here is a cobbled together section, sorry I had to cut a few different dumps together to keep out company IP information:

N: android=http://schemas.android.com/apk/res/android
  E: manifest (line=2)
    A: android:versionCode(0x0101021b)=(type 0x10)0x1e
      E: service (line=73)
        A: android:exported(0x01010010)=(type 0x12)0xffffffff
        E: intent-filter (line=74)
          C: ".9\n"

That C: ".9\n" is the line I've seen. No clue what its for.

jdarling commented 10 years ago

Oh and sorry about the catch(e)'s throwing Travis CI off, wouldn't have thought you would be using JSHint with IE mode :)

rubenv commented 10 years ago

IE mode?

jdarling commented 10 years ago

The error from Travis CI is "[L141:C26] W002: Value of 'e' may be overwritten in IE." based on my try/catch blocks only using catch(e). There should be a flag on JSHint that turns off IE only warnings, sorry don't use JSHint so I don't know what the flag is, that should be safe to turn off since apk-parser really couldn't be used in a browser.

rubenv commented 10 years ago

I just noticed that there isn't any .jshintrc in the repo. Let me add the one I usually use.

I don't really care about IE compatibility for Node.JS code, I do care about consistent code style :-)

rubenv commented 10 years ago

I just added a JSHint configuration. It should cause the linting to behave more reasonably.

From looking at your diff, you'll probably get a ton of new errors and warnings, mostly about whitespace. Sorry about that, but you'll have to fix it.

The patch is currently a mix of different styles and that's just plain ugly.

For instance, there's both function(response) { (space before accolade), function(err){ (no spaces at all) and function () { (the proper way to do whitespace and anonymous functions).

I'd love to merge this (good additions!), but we need to step up the code quality a bit first.