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

Refactor the modules #391

Closed
cburschka opened this issue May 28, 2016 · 2 comments
Closed

Refactor the modules #391

cburschka opened this issue May 28, 2016 · 2 comments
Assignees

Comments

@cburschka
Copy link
Owner

As outlined in #237 and #323, the eventual goal is to replace the current chat, ui, xmpp and visual globals with something that resembles MVC: A Cadence module that controls both the ui and the xmpp modules, where the latter is basically self-contained.

Error messages would be handled more like exceptions - thrown up to the caller rather than being printed on the screen when they happen.

The nucleus of the new Cadence module is what is currently chat.

cburschka added a commit that referenced this issue May 28, 2016
This begins the refactoring.
cburschka added a commit that referenced this issue May 28, 2016
Commands are no longer invoked directly. This will allow
restructuring the code internally.
cburschka added a commit that referenced this issue May 28, 2016
This adds a new public function, Cadence.addCommand(),
which can be used to register a particular command name with
a callback function.

All core commands are separately defined in a file called commands.js.
cburschka added a commit that referenced this issue May 28, 2016
cburschka added a commit that referenced this issue May 29, 2016
This patch introduces consistent exception handling.
Instead of printing errors themselves, all commands throw a
Cadence.Error type, which is then printed to the screen
by the calling function.

Hardcoded state-based command availability is replaced by Command.require,
which adds a simple boolean checker to the command. (The requirement
function can either return true or throw a Cadence.Error.)
In the same style, the parser is now added by .parse() instead of an argument
in the constructor.

The more complex deferral chains (admin, connect, configure, create, join)
have been entirely rewritten to create a semblance of readability.
Instead of anonymous functions, the chain links are now defined one by one
and then linked up at the end.
cburschka added a commit that referenced this issue May 29, 2016
Add a converter that turns an Error into a Cadence.Error,
so it can be displayed on the user interface.
@cburschka
Copy link
Owner Author

These last two patches have finally added some exception handling to cadence.

Once they get introduced to the xmpp code as well (and specifically the event handlers), the client will no longer "crash" and we will finally (finally) have fixed the legendary #163 bug.

@cburschka cburschka added this to the 1.12.0 - Trixie milestone May 29, 2016
@cburschka cburschka self-assigned this May 29, 2016
cburschka added a commit that referenced this issue May 29, 2016
This adds a StanzaError and ConnectionError type to the xmpp module.
The cadence commands catch these errors and don't need to traverse
the stanza itself anymore.

Includes some refactoring and other fixes.
cburschka added a commit that referenced this issue May 29, 2016
If we want to catch asynchronous errors with .catch(), then we need
Command.execute() to always return a promise - even if it ended
synchronously. To that end, wrap the return value in Promise.resolve().

(This will not catch synchronously thrown errors, which still require a
try...catch block.)
cburschka added a commit that referenced this issue May 29, 2016
If commands do asynchronous stuff without returning the promise,
then the exceptions thrown inside them cannot be caught.

Also, fixing two errors in the xmpp.StanzaError constructor.
cburschka added a commit that referenced this issue May 29, 2016
- reverse $.extend in handleError to avoid clobbering {command}.
- rename {name} to {room} in room selection menu.
- destructure Error object in fromError.
- return the promise chain in /admin.
- fix the /admin arg parser.
- fix the /create promise chain.
- return true after showing a server announcement, to keep the handler.
- [].each -> [].forEach.
- Use instanceof instead of .constructor, which is null/undefined safe.
@cburschka
Copy link
Owner Author

Done, as far as 2.0.0 is going to get at least.

Overhauling the xmpp module to give it contexts is something for the next version after that.

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

No branches or pull requests

1 participant