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

Open discussion about the next release of Krumo #15

Closed
scottchiefbaker opened this issue Oct 30, 2013 · 13 comments
Closed

Open discussion about the next release of Krumo #15

scottchiefbaker opened this issue Oct 30, 2013 · 13 comments

Comments

@scottchiefbaker
Copy link
Collaborator

@bobdenotter and I are planning on doing some major code refactoring and clean up. We hope to get Krumo in a better state, more readable state. We will also be looking at potential new features.

One thought I had was trying to detect a hex color code and displaying the actual color. Similar to how we detect unixtimes and translate them.

What else should Krumo have that it doesn't have today?

@bobdenotter
Copy link
Contributor

My primary goal isn't "more features". In it's current state Krumo does what it needs to do, and it really shouldn't grow into an abomination with countless features that nobody needs. At first i'd like to work on the current codebase, to ensure that the present functionality is in a good state, without adding any new functionality. In fact - if it's up to me - we could even cut out some dead code, while we're at it. :-)

@scottchiefbaker
Copy link
Collaborator Author

There is a TON of code in there that probably NEVER gets used. Specifically:

krumo::classes()
krumo::interfaces()
krumo::include()
krumo::functions()

etc...

I thought about removing them, but I didn't want to negatively affect users. I don't know if it's possible to make those part of an optional module or something. If we're looking at 1.0 release it might be a good time to cut them all together?

Basically removes lines 55 through 431. That's 26.5% of the code.

@donquixote
Copy link

Hi,
I am following this list with one eye.
I personally did a rewrite of krumo, which unfortunately for now is only available as a Drupal module.
https://drupal.org/project/krumong (think I mentioned it already somewhere)

Disclaimer: The comments here are all out of memory, and may not exactly represent what is actually going on in the module.

The main important thing I did there was to separate tree traversal (and "hive" management) from html output.
The rewrite has two classes for tree traversal, one "depth first" and one "breadth first".
Then it has a number of "theme" classes: One for the regular html output, one that reproduces var_dump() or print_r() etc.

I don't remember whether I actually implemented that, but I had the idea of a 3rd layer for value rendering. The idea was to have different plugins for different types of "special" data. This way you could have special displaying of colors, timestamps, instances of special PHP classes (e.g. some native classes that are special), etc, without bloating the main class.

I think this direction was definitely a good idea. You should do the same, and only then start with new features.

@donquixote
Copy link

Another thing i changed was to output json instead of html, and then use client-side javascript to construct the html structure and bind the events.
Without js and css, the krumo html is quite useless anyway.

@scottchiefbaker
Copy link
Collaborator Author

That's another thing I was thinking about, thanks for reminding me... We need some sort of CLI mode. Even if initially we just detect CLI mode and wrapper around var_dump() or print_r() that would be handy.

@scottchiefbaker
Copy link
Collaborator Author

We could also look at making the unstyled (broken/missing CSS) version not be TOTALLY horrible. That way if your CSS isn't readable or something else weird, the output you get is still SORT of usable.

@donquixote
Copy link

Would it be an acceptable option to let the user decide whether to output html or CLI ? E.g. krumo() spits out html, while Krumo::print_r() spits out print_r equivalent..
(not sure whether static methods or floating functions are the way to go here)

@scottchiefbaker
Copy link
Collaborator Author

We could certainly let the user pick the output type.

I think we should be "smart" and detect if the script is in CLI mode and output text mode unless the user specifies otherwise though.

@bobdenotter
Copy link
Contributor

detecting CLI isn't hard. We should do that, yes.

@scottchiefbaker
Copy link
Collaborator Author

I've created a "next" branch on my repo, and I've started work on some of the things discussed above.

Another thing I'd to see is a set of community agreed up defaults. I'd like to make the krumo.ini totally optional. I want the default krumo.ini to be 100% commented out. The only reason you change anything in there is if you want some non-default behavior.

I want Krumo to be as "zero-configuration" as possible. My goal is:

  1. Download
  2. Untar
  3. Fully functional Krumo

No configuration required unless you want something specific.

@bobdenotter
Copy link
Contributor

No, no, we should leave the default krumo.ini in, we should just change the behaviour of setting config.. It should work by overriding specific settings, instead of replacing all of the settings, when you call config manually.

@scottchiefbaker
Copy link
Collaborator Author

Yes definitely leave the config there, I just want all the options commented out (i.e. default). That ways it's less daunting for a use to look at changing things. Right now the config contained several default settings, like truncate length, and show version, and separater etc.

See:

https://github.com/scottchiefbaker/krumo/blob/next/krumo.ini

@bobdenotter
Copy link
Contributor

Maybe we should make a separate thread about this, but the config should not be commented out. Those are the defaults, in one convenient place. And then the user can override one or more or zero of those by calling the config function from their code.

We should provide the .ini file, with sensible settings, to use it "out of the box".

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

No branches or pull requests

3 participants