Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds native record encoding to the library #1

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

KAYLukas
Copy link

@KAYLukas KAYLukas commented Apr 7, 2014

Adds the possibility to encode records natively without using createDictWith to the library. The output is always lowercase.

Due to introspection to retrieve the contents of a record this method is somewhat slower than the original method.

KAYLukas added 22 commits April 7, 2014 14:58
Also various performance increases. Now uses python to perform the
parsing.
Better support for unicode: tests added
ASUnit for testing
Bug fix in encodeString — Escaping of quotes did not occur
Allows decoding of spaces
Sometimes applescript will not throw errors when casting a record to
text (which allows the user to see the record as raw bytes), a
workaround is in place.
Some bugs occurred when reading strings with single quotes, this is
fixed by letting python read from the stdin
Lastly the performance of encoding records is increased because its no
longer needed to json encode the string representation. Also when
encoding a list with records we use the encodeRecords method to reduce
overhead of calling encodeRecords multiple times.
New lines in records made decode crash.

Big strings > 256 are now handled by python, due to slow string
processing of AppleScript
@KAYLukas KAYLukas closed this Apr 23, 2014
@mgax
Copy link
Owner

mgax commented Jun 20, 2014

Hey, I missed this pull request, are you still interested in merging it?

@KAYLukas
Copy link
Author

Yeah sure

@KAYLukas KAYLukas reopened this Jun 25, 2014
@KAYLukas
Copy link
Author

Note: Issue #3 was already resolved in my branch.

@mgax
Copy link
Owner

mgax commented Jun 25, 2014

Ok, so this changes a lot of things, and it doesn't merge cleanly any more. Could you break it up into a few patches?

Also, why is it useful to call out to Python code?

@KAYLukas
Copy link
Author

Python was mainly because of performance reasons... I use string manipulation to transform a json string to applescript notation, but performing this manipulation in applescript turned out in a later stadium to be rather slow.

I can break it up into a few patches, but maybe implementing boolean support on my side is easier.

@winny-
Copy link
Contributor

winny- commented Jun 25, 2014

This is rather exciting. I have a few suggestions to make it even better, like an aged cheese(!).

Is it possible to break the python code out of json.applescript into a new file/module? This way Python can bytecompile the code only once instead of every time on execution (since the code is passed via the -c argument). It also makes it easier to read and modify.


I suggest adding a separate build target in the Makefile for ASUnit/ASUnit.scpt. As it's written right now, it will be compiled every time the test target is called instead of only when it needs to be built (first call into test or modifications made to ASUnit.applescript.

Something like this should fit the bill:

json.scpt: json.applescript
    osacompile -o json.scpt json.applescript

ASUnit/ASUnit.scpt: ASUnit/ASUnit.applescript
    osacompile -x -o ASUnit/ASUnit.scpt ASUnit/ASUnit.applescript

test: json.scpt ASUnit/ASUnit.scpt
    osascript tests.applescript

.PHONY: test

@KAYLukas
Copy link
Author

@winny-
I think that is a great option. Modifying the python code is now pretty annoying due to the indentation. But wouldn't it be then better to also compile it into a bundle? I thought I considered the splitting of python code a while back, but didn't want to create a bundle - for reasons I do not recall (might be because I was to lazy, or that I wanted a self containing file).

@mgax
Copy link
Owner

mgax commented Jun 27, 2014

@winny-

I suggest adding a separate build target in the Makefile for ASUnit/ASUnit.scpt.

That makefile doesn't work, make test dies with make: *** No rule to make target ASUnit/ASUnit.applescript', needed by ASUnit/ASUnit.scpt'. Stop.. But as it is, make test is really fast, why bother compiling ASUnit separately?

[edit] never mind, I was on the wrong branch.

@mgax
Copy link
Owner

mgax commented Jun 27, 2014

I can't merge ASUnit, it's GPL-licensed. But where is it used?

@KAYLukas
Copy link
Author

@mgax Pretty much all my testcases are in ASUnit.

Isn't it allowed to use GPL-tools for unit testing only, because it is not really part of the library?

@winny-
Copy link
Contributor

winny- commented Jun 29, 2014

IANAL: Using ASUnit means the test harness has to call into its code, suggesting that test cases are a derivative work and possibly subject to the GPL. It's ambiguous what the GPL allows for non-linking languages like AppleScript or Python.


I looked into how to create a script bundle with a non-AppleScript Resource file. The workflow is something like this:

  • osacompile -o json.scptd json.applescript — creates a Script Bundle (directory, similar to an Application tree); notice the .scptd file extension.
  • Install your Python script into json.scptd/Contents/Resources/
  • Now, you can Resolve the path in Applescript like this: path to resource "applescript_json_helper.py"

There is a id attribute on the script class, it is missing value if the given script is not in a bundle. This could be used to determine if the json script is indeed a plain .applescript file or a .scptd script bundle.

I hope this helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants