Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Initial implementation of modules #45

Merged
merged 4 commits into from
Jan 6, 2012
Merged

Initial implementation of modules #45

merged 4 commits into from
Jan 6, 2012

Conversation

gruehle
Copy link
Member

@gruehle gruehle commented Jan 5, 2012

This is a first pass at modularizing Brackets source code. When viewing the diffs, you probably want to add "?w=1" to the end of the URL to ignore white space.

Here is a link to the diff without whitespace: https://github.com/adobe/brackets/pull/45/files?w=1

@jasonsanjose
Copy link
Member

Should we consider following this http://requirejs.org/docs/jquery.html for using jQuery with modules?

};

// Define public API
// TODO: Once KeyMap is moved into separate module, export KeyBindingManager methods instead of the entire object
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure I understand this right: if we made KeyMap an "inner class" of KeyBindingManager, then this wouldn't be an issue, right? We'd assign public members of KeyBindingManager directly to the exports object, and one of those members would happen to be the KeyMap ctor?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we could do that (export the KeyMap ctor as a public property of KeyBindingManager), or we could make KeyMap a separate module. Either one would be fine.

@gruehle
Copy link
Member Author

gruehle commented Jan 6, 2012

(In reply to Jason's comment above):

The require-jquery file is a combination of require.js and jquery.js. They say that they regularly integrate jQuery, but we would be at the mercy of require.js to make sure our jQuery is up to date. I figured it was safest just to keep them separate.

@peterflynn
Copy link
Member

Looks good to me -- merging now.

peterflynn added a commit that referenced this pull request Jan 6, 2012
Initial implementation of modules
@peterflynn peterflynn merged commit 8e1d532 into master Jan 6, 2012
sedge pushed a commit to sedge/nimble that referenced this pull request Mar 12, 2015
sedge added a commit to sedge/nimble that referenced this pull request Mar 12, 2015
gideonthomas pushed a commit to gideonthomas/brackets that referenced this pull request Mar 13, 2015
sedge pushed a commit to sedge/nimble that referenced this pull request Mar 18, 2015
sedge pushed a commit to sedge/nimble that referenced this pull request Apr 20, 2015
gideonthomas pushed a commit to gideonthomas/brackets that referenced this pull request May 15, 2015
gideonthomas pushed a commit to gideonthomas/brackets that referenced this pull request May 20, 2015
sedge pushed a commit to sedge/nimble that referenced this pull request May 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants