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

Re-add Object/toString CSSColor implementation #31

Open
guns opened this issue Nov 15, 2013 · 4 comments
Open

Re-add Object/toString CSSColor implementation #31

guns opened this issue Nov 15, 2013 · 4 comments
Milestone

Comments

@guns
Copy link

guns commented Nov 15, 2013

Continued from: eb16db6#commitcomment-4614132

Commit eb16db6 included the following hunk:

-  #+clj Object #+cljs default
-  (toString [this]
-    (as-hex this)))
+  ;;Introduces an infinite loop when as-hex throws an exception
+  ;;Object
+  ;;(toString [this]
+  ;;  (as-hex this))

This was presumably done in the interest of ClojureScript compatibility, however, the implicit as-hex string conversion of CSSColor instances is very convenient:

(css [:elem {:box-shadow (str "0px 1px 1px " deep-blue)}])

In the interests of bringing back this feature I propose either:

  1. Change as-hex and related functions to not throw Exceptions in favor of using default values and clipping out-of-bound values
  2. Wrap (as-hex this) in a try catch and return a default string.

This is not a major issue, of course, but it would be nice.

Thanks!

@jeluard
Copy link
Collaborator

jeluard commented Nov 15, 2013

Right I caught an infinite loop while migrating this part to ClojureScript. It's not strictly needed for ClojureScript compatibility but just commented it waiting for a better solution.

Have you tried something like {:box-shadow [[0 (px 1) (px 1) deep-blue]]} ? It looks like it does what you expect.

@guns
Copy link
Author

guns commented Nov 15, 2013

Oh, wrapping with another vector suppresses commas? Very useful, I should pay more attention.

I won't mind if you close this issue, but if you are looking to reinstate a toString we could keep it open.

@jeluard
Copy link
Collaborator

jeluard commented Nov 15, 2013

I'll let @noprompt comment on that, I personnaly don't really have an opinion :)

@noprompt
Copy link
Owner

Oh, wrapping with another vector suppresses commas?

Yes [1 2 3] => 1, 2, 3 and [[1 2] [3 4]] => 1 2, 3 4. Also, I agree it is/was nice. In general I think the garden.color namespace is the most in need of some attention (next to the compiler). I've been a bit on the busy side recently (just started a new job) and without an internet connection at home (the horror) so if anyone wants to look in to this that'd be awesome. Pull requests are always welcome and I know both of you are skilled programmers. :-)

@noprompt noprompt added this to the v2.0.0 milestone Feb 5, 2014
noprompt added a commit that referenced this issue Nov 29, 2016
This patch rethinks the architecture of Garden in terms of well
defined boundaries between parsing and compiling. Instead of
directly compiling objects into CSS, Garden will first parse the objects
into a CSS AST which can then be consumed by the compiler. This
architecture is closer to what one might expect to find in a
programming language implementation setting. This new architecture
yields greater flexibility to introduce new features and semantics
while vastly simplifying the work of the compiler.

As a result many requested features and solutions to bugs have now
been either implemented, solved, or are now trivial to implement or
solve.

Changelog updates forthcoming.

Closes #31
Closes #109
Closes #115
Closes #116
Closes #119
Closes #120
Closes #121
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

3 participants