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

Added Debug and Xray npm packages. #129

Merged
merged 5 commits into from
May 18, 2016
Merged

Added Debug and Xray npm packages. #129

merged 5 commits into from
May 18, 2016

Conversation

ciscoheat
Copy link

Two pretty useful packages. Debug is nice and simple: https://www.npmjs.com/package/debug

X-ray is more advanced, and I had to create a separate version of the request driver it uses, I hope that's ok.

Finally, since both packages uses a function with properties, I'm using an abstract for the implementation. The @:callable metadata makes it possible, with that it's possible to call the underlying function directly. Nice stuff!

@ciscoheat
Copy link
Author

I'm about to add one more package to this one, brb

@clemos
Copy link
Owner

clemos commented Mar 8, 2016

I was reviewing this PR, trying to figure a way to simplify it if possible...

Fixed abstract enum in Soap.hx
@ciscoheat
Copy link
Author

Great if you can, I've tried a couple of things but couldn't make it better than this. The problem is how to call properties on a function. (That abstract solves quite nice, imo)

@clemos
Copy link
Owner

clemos commented Mar 8, 2016

Off the top of my head, regarding debug, I think something like that might be better / simpler (not even sure if it works):

extern class Debug
implements npm.Package.Require<"debug","^2.2.0">
{
   @:selfCall
   public function new(name:String):Void;
   // special function
   public inline function call( data : Dynamic, ?infos : haxe.PosInfos ) {
      var args = [data];
      if( infos != null && infos.customParams != null ) {
          args = args.concat( infos.customParams );
      }
      untyped this.apply( this, args );
   }

   // not sure about this one, but why not
   // it could make it possible to `debug.log = haxe.Log.trace`
   public var log: Dynamic -> ?haxe.PosInfos -> Void;
}

See http://old.haxe.org/doc/cross/trace

What do you think ?

@ciscoheat
Copy link
Author

I'll take a look!

@ciscoheat
Copy link
Author

Now I remember, there is a static log var on the Debug class as well, that's why the solution is a bit more complicated.

@clemos clemos closed this Mar 9, 2016
@clemos clemos reopened this Mar 9, 2016
@clemos
Copy link
Owner

clemos commented Mar 9, 2016

Sorry, misclicked

@ciscoheat
Copy link
Author

Just commited a version that supports haxe.Log.trace in both directions. Thanks for the suggestion!

Added commonmark Markdown renderer.
@ciscoheat
Copy link
Author

Expanded the Jade class now, and added the Commonmark markdown package.

}

public static inline function enable(name : String) : Void {
untyped Lib.require("debug").enable(name);
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think you need inline or this trick, do you ?

Copy link
Author

Choose a reason for hiding this comment

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

I wish it was that simple. There are many things that should be fulfilled here:

  1. Easy instantiation: var debug = new Debug("myapp")
  2. Easy usage: debug("Working")
  3. Easy configuration: debug.log(haxe.Log.trace)
  4. Easy static configuration: Debug.log(someOtherLog)

It seems not possible to fulfill all these at the same time. If I use construct and an abstract class, 2-4 can be fulfilled, keeping as close to the original API as possible.

Copy link
Owner

Choose a reason for hiding this comment

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

All 4 should be possible without inline, since Debug and DebugInstance are two different classes, shouldn't it ?

@ciscoheat
Copy link
Author

So, any chance to get this pulled in? :)

/**
* A more powerful version of the default http-driver.
*/
class XrayHttpDriver
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not too much in favor of adding logic in externs :S

@clemos
Copy link
Owner

clemos commented Mar 31, 2016

Sorry for the delay; I just added more comments :S

@ciscoheat
Copy link
Author

I can remove the XrayHttpDriver, no problem. But I cannot see, in both the case of xray and debug, how to make an extern that can call itself directly? @:selfCall is a solution, but then you need to call a method on the object, and what name should that method have, compared to just calling it (with an abstract marked with @:callable)?

This was a nice metadata reference for those, btw: https://haxe.io/releases/3.2.0/#Rest-EitherType-selfCall-callable

@clemos
Copy link
Owner

clemos commented Apr 7, 2016

The problem is not so much with new, construct, @:selfCall.
My point is just with replacing :

    public static inline function enable(name : String) : Void {
        untyped Lib.require("debug").enable(name);
    }
    public static inline function disable() : Void {
        untyped Lib.require("debug").disable();
    }
    public static inline function log(log : Either<DebugInstance, DebugHaxeLogTraceFunction>) : Void {
        untyped Lib.require("debug").log = log;
    }

with

    public static function enable(name : String) : Void;
    public static function disable() : Void:
    public static var log : Either<DebugInstance, DebugHaxeLogTraceFunction>;

when possible.
But once again, maybe I'm missing something ?

@ciscoheat
Copy link
Author

Ok, maybe that will work! I'll make an attempt.

Added PrettyJson.hx
@ciscoheat
Copy link
Author

I think this is as good as it gets. Xray makes things very complicated, in plain js:

require('x-ray')()('selector...').paginate(3)(function(err, result) { ... })

So I had to make two @:selfCall methods, x and done, resulting in this code:

var xray = new Xray();

xray.x('https://google.com', {
    main: 'title',
    image: xray.x('#gbar a@href', 'title')
})
.done(function(err, result) {
    trace(result);
});

Debug was easier with the static methods, I'm not very happy about the construct method, but I don't see any other way as it is now.

Added a bonus too, a much simpler prettyjson package. :)

@ciscoheat
Copy link
Author

If you have suggestions for other names than x and done, let me know.

@clemos
Copy link
Owner

clemos commented May 18, 2016

Ok let's merge it as is :p

@clemos clemos merged commit d53b146 into clemos:develop May 18, 2016
@ciscoheat
Copy link
Author

Thanks! it's not a total disaster I hope. :)

@ciscoheat ciscoheat deleted the npm branch May 19, 2016 11:57
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.

2 participants