Skip to content

Conversation

@elkuku
Copy link
Contributor

@elkuku elkuku commented Apr 21, 2017

Summary of Changes

The goal of this PR is to refactor the debug plugin (currently more than 2.000 lines of code) into smaller pieces.
An external library is used to display a floating debug bar. See a demo

Testing Instructions

run composer install after switching to this branch
I have not included any vendor files or modified composer files here to make reviewing easier. Is that correct?

Turn on debug and/or language debug and play with the various options in plugin configuration.
Do not turn on deprecation logging since the logger is using currently deprecated API creating an infinite loop...

Expected result

A beautiful debug toolbar 😉

Actual result

A not so beautiful looong thingy, fixed to the very bottom of the page.

Documentation Changes Required

Alot 👅

Extended Description

While working on a Symfony project, I was amazed by the debug bar they offer and I thought about the old Joomla! plugin...

Integrating the Symfony bar would require a lot of dependencies from Symfony, probably including the Symfony Application class. While this would be amazing, I suspect that this is not gonna happen any time soon...

So I started playing with maximebf/debugbar which claims to be inspired by the Symfony debug bar, but for "non Symfony" projects ;)

Thise resulted in some DataCollector classes which, apart from collecting data, are responsible for bringing the collected data to something "serializable".

This data is then serialized and written to disk for later analysis.

After a thousand words, here are some screens:
Note: Since the "old" code is still in place, you will see the "old" output as well as the "new " output in the screenshots.

Session

Currently the session display in debug plugin is broken.
2017-04-18-121246_1366x768_scrot

Profile

2017-04-21-133031_1366x768_scrot

Queries

Only the database queries are displayed. Extended display should happen on a dedicated page.
2017-04-18-121436_1366x768_scrot

Deprecation logging

Seems currently broken since file loggers are already using deprecated API, creating funny infinite loops...
2017-04-21-171552_1366x768_scrot

Language stuff

Should look as before, just more readable and compact.

Loaded

2017-04-21-132535_1366x768_scrot

Errors

2017-04-18-121557_1366x768_scrot

Untranslated

2017-04-18-121539_1366x768_scrot

Hidden errors

While playing here I discovered a query error that is so much eaten up and swallowed that it does not appear in any way on the standard output. I would bet that this is causing a bug...
2017-04-21-171402_1366x768_scrot

Mobile ready

screenshot_20170426-010917

Tested on a 5'' screen with fat fingers and it worked pretty well.


Happy debugging =;)

@zero-24
Copy link
Contributor

zero-24 commented Apr 21, 2017

run composer install after switching to this branch
I have not included any vendor files or modified composer files here to make reviewing easier. Is that correct?

@elkuku please include the composer files in your commit and place them in the vendor folder. As we are shipping the vendor files with the CMS ;)

@elkuku
Copy link
Contributor Author

elkuku commented Apr 21, 2017

@zero-24 that would be over 2.000 files, making it almost impossible for reviewing. Please reconsider..

BTW...

place them in the vendor folder.

I believe you mean /libraries/vendor?

@zero-24
Copy link
Contributor

zero-24 commented Apr 21, 2017

Yes that folder. Hmm can you .gitignore files out that are not needed?

As without shipping than with the cms we cant use that lib in the current architecture.

@mbabker
Copy link
Contributor

mbabker commented Apr 22, 2017

Run "Composer install", "Composer update package", "Composer install --no-dev" and you'll be good.

@elkuku
Copy link
Contributor Author

elkuku commented Apr 22, 2017

@zero-24 I understand that vendor files have to be included in the final package, I just thought it would be easier to review...

@mbabker Of course your're right (as usual), most of the files are dev dependencies...

Anyway, I created a new branch, you may review the diff here, and if you find it reviewable, I'll merge it in here...

@Bakual
Copy link
Contributor

Bakual commented Apr 22, 2017

The needed dependencies should be "reviewed" as well anyway. Not exactly codewise but personally I want to see what you're going to pull in.
For example If it needs 5000 lines of code in various dependencies to replace the 2000 lines in our debug plugin, then we at least should question it. 😊

@brianteeman
Copy link
Contributor

Files not lines

@Bakual
Copy link
Contributor

Bakual commented Apr 22, 2017

The 2000 files are with dev stuff which we don't need. So the actual file count is lower. I just did a guess with the 5000 lines. Looking at the other branch, it's actually 20'000 lines we add. So imho there has to be a very good reason to add all those dependencies just for a debug plugin.
But that is my personal opinion.

@wilsonge
Copy link
Contributor

wilsonge commented Apr 22, 2017

Can we use the namespacing convention listed at https://volunteers.joomla.org/teams/joomla-4-architecture/reports/115-joomla-4-architecture-sprint-odense-dk please. So Joomla\Plugin\System\Debug please :)

@wilsonge
Copy link
Contributor

I like this concept overall :)

$this->app = JFactory::getApplication();
}

if ('com_profiler' == $this->app->input->get('option'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have something for community builder here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LOL I could have sworn that I heard com_profiler before... sorry I almost left the Joomla! community :(

And no, it's not community builder I have in mind here but a component that will display the extended "profile" information that the debug bar is unable to display (due to its size).

It has slipped in here and will be removed.

@elkuku
Copy link
Contributor Author

elkuku commented Apr 23, 2017

@Bakual and @brianteeman

The needed dependencies should be "reviewed" as well anyway. Not exactly codewise but personally I want to see what you're going to pull in.

I thought the change in composer.json might be enough, but you are right. So here are some more infos on files and lines 😉

php-debugbar

~/r/joomla-cms (debug-bar) $ phploc libraries/vendor/maximebf/
phploc 3.0.1 by Sebastian Bergmann.

Directories                                          7
Files                                               45

Size
  Lines of Code (LOC)                             6328
  Comment Lines of Code (CLOC)                    2123 (33.55%)
  Non-Comment Lines of Code (NCLOC)               4205 (66.45%)
  Logical Lines of Code (LLOC)                    1179 (18.63%)

The library has one dependency:

symfony vardumper

~/r/joomla-cms (debug-bar) $ phploc libraries/vendor/symfony/var-dumper/
phploc 3.0.1 by Sebastian Bergmann.

Directories                                         10
Files                                               56

Size
  Lines of Code (LOC)                             7620
  Comment Lines of Code (CLOC)                    1074 (14.09%)
  Non-Comment Lines of Code (NCLOC)               6546 (85.91%)
  Logical Lines of Code (LLOC)                    1708 (22.41%)

This is a pretty cool replacement for var_dump() and can be used also by 3PDs.

@elkuku
Copy link
Contributor Author

elkuku commented Apr 23, 2017

@wilsonge

I like this concept overall :)

Big Thanks for this 😉

Now on the namespacing thing.. I'd love to follow any conventions but I wasn't aware of the existence..
Could you give me some advice on how to accomplish this?
I mean I would have to create the folder(s) JROOT/plugins/system/debug/Joomla/Plugin/System/Debug and register the plugin root as the namespace root?
That seems odd :(

@brianteeman brianteeman modified the milestone: Joomla 4.0 Jun 8, 2017
@Fedik
Copy link
Member

Fedik commented Nov 13, 2017

guys what need to do to get this accepted?
it seems a great improvement

@dgrammatiko
Copy link
Contributor

@Fedik
Copy link
Member

Fedik commented Nov 13, 2017

I would go with: https://underground.works/clockwork/

only drawback, it forces me to install the browser extension 😄

@brianteeman
Copy link
Contributor

or use the web UI http://your.app/__clockwork

@mbabker
Copy link
Contributor

mbabker commented Nov 13, 2017

They need to add a LICENSE file to that repo. The note of the license in the Composer manifest isn't enough.

@Fedik
Copy link
Member

Fedik commented Nov 13, 2017

or use the web UI http://your.app/__clockwork

did not seen this part :)

@brianteeman
Copy link
Contributor

@mbabker I wondered about that

@brianteeman
Copy link
Contributor

It does state MIT in the composer file

@Fedik
Copy link
Member

Fedik commented Nov 14, 2017

sooo, I have tested both Debug bar and Clockwork

In 2 words, for Joomla! I would choose "Debug bar".

Here is some notes/comparison:

Debug bar

pros:

  • client side rendering (based on own renderer)
  • possible to view debug history (not implemented in current pull)
  • possible to create a custom data handlers
  • possible to create a custom client side widgets
  • not bad documentation

cons:

  • I didn't found anything critical

Clockwork

pros:

  • client side rendering (based on AgularJS 1.5)
  • possible to view debug history
  • possible to create a custom data handlers

cons:

  • client side depend from browser extension or need to adapt /Clockwork/Web/public
  • not possible to create a custom client side widgets, except we write custom client rendering
  • documentation does not exist (I not found)

So I think Clockwork is a nice toy or when someone have a time to fork and adapt it,
for Joomla! I would choose Debug bar.

@Fedik
Copy link
Member

Fedik commented Nov 18, 2017

@elkuku I have fixed the conflicts https://github.com/Fedik/joomla-cms/tree/debug-bar

Now I think, it would be more easy to write new plugin than rewrite existing 😄

@laoneo
Copy link
Member

laoneo commented Apr 9, 2018

I would welcome to have a more modern debug bar for Joomla. Can you bring that pr into a state for reevaluation please. Thanks!

@wilsonge
Copy link
Contributor

wilsonge commented May 2, 2018

There's no action happening here. i'd love to see something like this make it into 4.0 - but I guess it's not happening in this PR - so I'm going to close this for now. Of course if it's comes back in sync we can reopen and evaluate :)

@wilsonge wilsonge closed this May 2, 2018
@elkuku
Copy link
Contributor Author

elkuku commented May 11, 2018

This has been updated

@@ -1,3 +1,3 @@
source 'http://rubygems.org'
source 'https://rubygems.org'

Choose a reason for hiding this comment

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

Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.

@elkuku
Copy link
Contributor Author

elkuku commented May 11, 2018

Sorry but it seems that something got wrong with the last merge... I'd like to close and redo this PR but unfortunately I am unable to close it....

@elkuku elkuku mentioned this pull request May 12, 2018
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.