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

Meaningful toString implementation #265

Closed
raimohanska opened this issue Oct 19, 2013 · 12 comments
Closed

Meaningful toString implementation #265

raimohanska opened this issue Oct 19, 2013 · 12 comments
Milestone

Comments

@raimohanska
Copy link
Contributor

@raimohanska raimohanska commented Oct 19, 2013

How about a toString() function that would return something like this:

combineAsArray([constant(a),constant(b)])

or

take(merge(later(1, "lol"), later(2, "wat")), 1)

A quick implementation exists at 30ae4aa. And when I say quick I mean that all tests for toString are missing at the moment.

@raimohanska
Copy link
Contributor Author

@raimohanska raimohanska commented Oct 20, 2013

In fd95c4b, I've implementated and tested a lot of toString implementations... Still some combinators left.

My feeling is that this might be worth the effort :)

@raimohanska
Copy link
Contributor Author

@raimohanska raimohanska commented Oct 29, 2013

Most of the work is already done in the branch feature/dependency-graph-dispatch.

Todo: specs for the rest of the combinators.

@raimohanska
Copy link
Contributor Author

@raimohanska raimohanska commented Oct 30, 2013

I'm planning to include this in release 0.7.0. Help is appreciated!

@raimohanska
Copy link
Contributor Author

@raimohanska raimohanska commented Nov 1, 2013

Whoa. Just squeezed the toString implementations in.

So, this stuff is ready for release, and resides currently in the feature/dependency-graph-dispatch branch.

@raimohanska
Copy link
Contributor Author

@raimohanska raimohanska commented Nov 3, 2013

This is actually a comeback of #107.

@dnalot @hden @RoboTeddy any comments? Should we include .inspect too?

@raimohanska
Copy link
Contributor Author

@raimohanska raimohanska commented Nov 3, 2013

Hmm.. When running in CoffeeScript, the new toString doesn't seem to work:

var Bacon=require("./dist/Bacon")
coffee> Bacon._.toString({a:"b"})
'[object Object]'

This is weird, as the tests pass quite well and they are done in coffee too.

@hden
Copy link
Contributor

@hden hden commented Nov 4, 2013

It seem to be only happing in the REPL. Perhaps its a compiler issue?

# test.coffee
'use strict'
Bacon = require "./dist/Bacon"
console.log Bacon._.toString({a:"b"})
# shell
$ coffee test.coffee 
{a:b}
# REPL
> Bacon = require "./dist/Bacon"
...
> console.log Bacon._.toString({a:"b"})
[object Object]
@philipnilsson
Copy link
Member

@philipnilsson philipnilsson commented Nov 4, 2013

Yeah, I think it's Node.js, see e.g.
http://stackoverflow.com/questions/7428235/how-to-print-json-object-content-in-node-js
How
come it's an object, not a string?

On Mon, Nov 4, 2013 at 2:10 AM, Hao-kang Den notifications@github.comwrote:

It seem to be only happing in the REPL. Perhaps its a compiler issue?

test.coffee'use strict'Bacon = require "./dist/Bacon"console.log Bacon._.toString({a:"b"})

shell

$ coffee test.coffee
{a:b}

REPL

Bacon = require "./dist/Bacon"
...
console.log Bacon._.toString({a:"b"})
[object Object]


Reply to this email directly or view it on GitHubhttps://github.com//issues/265#issuecomment-27659933
.

@hden
Copy link
Contributor

@hden hden commented Nov 21, 2013

It seems that this problem is fixed in the current master of coffee-script. Ready to close this issue?

@raimohanska
Copy link
Contributor Author

@raimohanska raimohanska commented Dec 19, 2013

The implementation is now in feature/0.7 and will be included in the 0.7.0 release.

@raimohanska
Copy link
Contributor Author

@raimohanska raimohanska commented Dec 19, 2013

One more thing: should we support .inspect() also?

I'm thinking about supporting toString/inspect for the following:

  • EventStream
  • Property
  • Bus
  • Event: Initial | Next | Error | End
  • Some | None

Should have tests for all of the included objects/classes.

@raimohanska
Copy link
Contributor Author

@raimohanska raimohanska commented Dec 19, 2013

Added a name method as @rassie suggested in #273.

Also implemented inspect for nice node output.

Now you can

> B.once("1").name("one")
one
> B.once(1).name("stuff").take(1)
stuff.take(1)

I'm done with this one!

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.