Skip to content

lint, update namespace/debug to be plugins, and their logic #12

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

Closed
wants to merge 3 commits into from
Closed

lint, update namespace/debug to be plugins, and their logic #12

wants to merge 3 commits into from

Conversation

tunnckoCore
Copy link
Contributor

  • should removed - not used anywhere.
  • mocha require removed - blocks us, we all have it globally.
  • test script to gulp, maybe would revert it, but it's a good thing anyway
  • update and add new tests for the debugging stuff
  • app._namespace is more stable now
  • add app._debugNamespace, because we can't just overwrite _namespace from debug plugin, or we can it would be unnecessary bonus job.
  • no need of Constructor.debug thingy and then calling it from app's constructur with debug(this)
  • everything is handled and works automagically, you can't believe, lol
  • battle tested, lol - meaning, local verb and any of the others works with it
  • the instance debug method accept string arguments, and also support splitting by :
  • add built-in debug methods e.g. this.debug.helper, this.debug.view, etc
  • builtin methods does not accept namespaces, because they are namespaces
  • if you want another namespace just use app.debug.my = app.debug('one')
  • you can control the built-ins, they also can be overwritten if the APP add the plugin manually
  • and yes, i think namespace should be separate plugin
  • and yes, there are a little diff and logic between _debugNamespace and _namespace

example.js

'use strict';

var Base = require('./index');

function Templates() {
  Base.call(this);
  this.is('templates');
}
Base.extend(Templates);

function Assemble() {
  Templates.call(this);
  this.is('assemble');
}
Templates.extend(Assemble);

function Generate() {
  Assemble.call(this);
  this.is('generate');
}
Assemble.extend(Generate);

function Verb() {
  Generate.call(this);
  this.is('verb');
}
Generate.extend(Verb);

var verb = new Verb();

console.log(verb._namespace);
//=> base:templates:assemble:generate:verb

var one = verb.debug('one');
one('debugging with %s Mike Reagent', 'Charlike');
//=> base:templates:assemble:generate:verb:one debugging with Charlike Mike Reagent

verb.debug.helper('loading foo helper');
//=> base:templates:assemble:generate:verb:helper loading foo helper

verb.debug('foo:bar', 123, 'baz:qux', 'zaz')('hello world');
//=> base:templates:assemble:generate:verb:foo:bar:baz:qux:zaz hello world

var templates = new Templates();
templates.debug('one:two', 'three:four')('okey, that is awesome');
//=> base:templates:one:two:three:four okey, that is awesome

I can PR to base-debug, templates, assemble, generate and verb to get the change (it would be just removeing the debug(this) thing from their constructors). And we should create base-namespace.

@@ -1,10 +1,7 @@
'use strict';

require('mocha');
require('should');
Copy link
Collaborator

Choose a reason for hiding this comment

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

we don't all have mocha globally, that's a bad practice. users shouldn't be forced to install it with -g

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will also work if it is installed locally when just npm install && npm test, without requiring it.

I don't know how much it is a bad practice, but hate to waste time to install shitty-mitty-bitty-biggy modules every time i'm going to test or pr to some project.
"every time i'm going to test" meaning, that there are many project that just have one devDependency and it is some testing framework.

But yea, preferences, religious things.. anyway. :)

edit: Another question is that that mocha is bad in general, exactly because this globals. I'm agree here - that is bad practice. That's why there is absolutely better and smaller testit, which is almost drop-in replacement for mocha. That's why i created assertit on top of testit with few lines enhancement and using it everywhere from one year or so.

@jonschlinkert
Copy link
Collaborator

as I mentioned in a comment, I think some of this belongs in base-debug, but in general, this is great!

Before we do that though, what are some other options for initializing custom namespaces like the ones in templates? when @doowb and I implemented that we were on screenhero, and our thinking was that it was a "one-off" situation. meaning that we would only need to do that in templates - but that was wrong obviously.

There are lots of different ways we can do this. We were planning on removing most of the debug code from base - but not all of it.

Once we got the debug stuff working in a way that seems easy to maintain and works the way we wanted it to, we were going to implement basic, "auto-namespaced" app.debug in base, then add additional methods and features in base-debug, so that implementors could customize how debug works on an app-by-app basis.

thougths?

@jonschlinkert
Copy link
Collaborator

I can PR to base-debug, templates, assemble, generate and verb to get the change (it would be just removeing the debug(this) thing from their constructors). And we should create base-namespace.

just saw that line, sorry! yeah that would be great. I can create base-namespace and add you to it and npm. I only ask that you not publish to npm without peer review. @doowb and I are usually on screenhero or ask each other for peer review on slack or on an issue. with some repos we don't ask for that, but with something like this that would break the world if there was an error.... => peer review <= thx!

@tunnckoCore
Copy link
Contributor Author

I only ask that you not publish to npm without peer review.

Haha. Yea, yea, no problem, i'm not greedy.

We were planning on removing most of the debug code from base - but not all of it.

Mm, yes? If you talking for the two plugins in utils, yes. I intentionally set them here and PR here - for review and to start from somewhere - it was the easier way, get it work here, get some thoughts from you, and get them out to separate plugins and use them here again.

we were going to implement basic, "auto-namespaced" app.debug in base,

It's done, buddy :D All the user need is to use the .is, and that's the cool thing to use the this.use from any other method, lol. And it is "very basic", because it is plugin (plugin architecture).

edit: okey, want PR to base-debug it's ready locally, just need git push.
edit2: actually, just hit of the PR button, lol.

@tunnckoCore
Copy link
Contributor Author

Btw, also the cool thing and idea behind this second namespace propery app._debugNamespace is that - this._namespace contains only the namespace of the app, not added debug namespaces, and the this._debugNamespace contains the whole thing - this is related to base-repos/base-namespace#1

@jonschlinkert
Copy link
Collaborator

this._namespace contains only the namespace of the app

okay, I was wondering... to clarify then:

  • this._namespace is essentially still the same (as before your pr), and would be something like base:templates:assemble:generate:verb?
  • this._debugNamespace would append something like :helpers?

@tunnckoCore
Copy link
Contributor Author

Yes, it works exactly in that way. Mm, let me look the tests again - i'm almost sure.

@tunnckoCore
Copy link
Contributor Author

Yes, asbolutely!

that's two of the base-debug tests

  it('should take a namespace', function() {
    var debug = app.debug('bar');
    debug('debugging %s', 'whatever');
    assert.equal(app._namespace, 'base');
    assert.equal(app._debugNamespace, 'base:bar');
  });

  it('should take a complex namespace', function() {
    app.debug('bar:one', 123, 'foo:qux:two', 'four', 'five:six')('hello world');
    //=> base:bar:one:foo:qux:two:four:five:six hello world

    assert.equal(app._namespace, 'base');
    assert.equal(app._debugNamespace, 'base:bar:one:foo:qux:two:four:five:six');
  });

where, if you are in assemble, _namespace would be base:templates:assemble and debugNamespace would be base:templates:assemble:bar:one:foo:qux:two:four:five:six

edit: so commented _suffix would be easy to implement, as you can see.

while (i < len) {
var ns = namespaces[i++];
app.debug[ns] = debugFactory.call(app, ns);
app.debug[ns].color = app.debug.color;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this line is working the way you think. Since app.debug is acting as a debugFactory instead of a debugger method, it doesn't have a .color on it.

If you're going the approach that a user needs to do app.debug()('this is a message') to access the default debugger, then I think that debugger needs to be cached with the namespace and the color from that can be used on the children to match the application color.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this line is working the way you think.

Haha, actually yea, i noticed it.

If you're going the approach that a user needs to do

It's intentional, to be used as compose thingy for other namespaces (methods on app.debug.*)

@tunnckoCore
Copy link
Contributor Author

God! New commented approach works as expected. Changed in no minute, that's awesome.

Okey, i can start with PR to base-debug, then base-namespace and we can clean this pr?

tunnckoCore referenced this pull request in tunnckoCore/base-debug Feb 29, 2016
@jonschlinkert
Copy link
Collaborator

i can start with PR to base-debug, then base-namespace and we can clean this pr?

sounds good thx

@jonschlinkert
Copy link
Collaborator

K I'm closing this pr since we discussed other options with base-debug and base-namespace. I also want to simplify the Base API now that we've had lots of time to get familiar with using it and what we need it to do

@tunnckoCore
Copy link
Contributor Author

Yea, I'm trying to come to base-debug and base-namespace. I hope it will be this week. I have them locally.

@jonschlinkert
Copy link
Collaborator

Hey no worries, take your time!

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