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

Adding Macroptimize Package #3879

Merged
merged 3 commits into from Dec 7, 2014
Merged

Adding Macroptimize Package #3879

merged 3 commits into from Dec 7, 2014

Conversation

bigcakes
Copy link
Contributor

@bigcakes bigcakes commented Dec 6, 2014

No description provided.

@FichteFoll
Copy link
Collaborator

This will be problematic for downloaded packages on ST3 because they are kept within zip archives and not as a folder on the filesystem. You can either add ST2- and ST3-specific operations to this package or, for simplicity, restrict the package to ST3.

For ST3 you should use the sublime.find_resources(pattern) API (which doesn't exist in ST2) that will also find archived packages. You can then open these with sublime.load_resource(path).
When executing macro files you indeed have to use forward slashes on all OSes.

For some tips regarding your code, I recommend to use collections.namedtuple for your macro class (and also to Capitalize that since it's a Python convention).
You can format strings (e.g. line 31) in two different ways: "%d %s" % (index, macro.prettyText) or "{index} {macro.prettyText}".format(index=index, macro=macro) (or "{0} {1.prettyText}".format(index, macro)).

@bigcakes
Copy link
Contributor Author

bigcakes commented Dec 7, 2014

Thanks a lot for the suggestions, @FichteFoll! I made all the changes to the package, the code is a lot cleaner now. I also changed the package to be ST3 only, however in doing that I quickly pushed the change with a space, which broke the build. I fixed that, but honestly don't know how to correctly squash the commits, and didn't want to mess up origin.

@FichteFoll
Copy link
Collaborator

It's only one redundant comment so it's okay. But you're right, since pull requests are opened from the origin repo in a separate branch you would need to force-push in order to squash commits in a pushed pull request.

However, your latest tag is not a valid semantic version. You have to change that to 0.2.0.

FichteFoll added a commit that referenced this pull request Dec 7, 2014
Adding Macroptimize Package
@FichteFoll FichteFoll merged commit e82be04 into wbond:master Dec 7, 2014
@bigcakes
Copy link
Contributor Author

bigcakes commented Dec 7, 2014

Thanks a lot for all the help! I changed the tag.

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.

None yet

2 participants