Skip to content

5.x: remove deprecations#935

Merged
LordSimal merged 7 commits into
5.xfrom
5.x-remove-deprecations
May 5, 2023
Merged

5.x: remove deprecations#935
LordSimal merged 7 commits into
5.xfrom
5.x-remove-deprecations

Conversation

@LordSimal

@LordSimal LordSimal commented Apr 30, 2023

Copy link
Copy Markdown
Contributor

Haven't fully tried it with a test app but this removes the ToolbarHelper::dump() method which was marked as deprecated.

The tests working with SimpleXmlElement created some problems for me locally since the JS contained in the Debugger Header did not parse well as XML. Therfore I changed the test implementation to use DOMDocument.

Update: Nope, there are definitely some problems still present.

@LordSimal LordSimal added this to the 5.x milestone Apr 30, 2023
@LordSimal

LordSimal commented Apr 30, 2023

Copy link
Copy Markdown
Contributor Author

I am probbaly misunderstanding things here but since we dropped ToolbarHelper::dump() one has to use ToolbarHelper::dumpNodes()

This one requires an array of nodes of type \Cake\Error\Debug\NodeInterface which can be created like

$nodeArray = [Debugger::exportVarAsNodes($request->getQueryParams())];

because Debugger::exportVarAsNodes() will always be 1 root Node containing all child nodes.

Therefore I had to do stuff like

'query' => [Debugger::exportVarAsNodes($request->getQueryParams())],
'data' => [Debugger::exportVarAsNodes($request->getData())],
'cookie' => [Debugger::exportVarAsNodes($request->getCookieParams())],
'get' => [Debugger::exportVarAsNodes($_GET)],

which feels wrong to me because it will created an output like this:
image

I seem to be missing something here so that we can also dump a single node. Should I just create that method in the ToolbarHelper?

@markstory

Copy link
Copy Markdown
Member

Should I just create that method in the ToolbarHelper?

That seems like the correct approach. Accepting a list or single node in the existing method could also be a good approach.

@LordSimal
LordSimal force-pushed the 5.x-remove-deprecations branch from 6a7c41d to 94c9df1 Compare May 1, 2023 09:08
@LordSimal

LordSimal commented May 1, 2023

Copy link
Copy Markdown
Contributor Author

This looks better
image

Now there is only the issue of the duplicate copy button but I think this is something we need to fix in the core.

@markstory

Copy link
Copy Markdown
Member

Looks good!

@LordSimal
LordSimal force-pushed the 5.x-remove-deprecations branch from 1c99b05 to 5e8c497 Compare May 5, 2023 11:02
@LordSimal

Copy link
Copy Markdown
Contributor Author

The duplicate copy buttons where happening because the debug_kit and cake core adds a separate div with the class cake-debug. But debug_kit doesn't need to do that anymore.

image

The copy button gets added to all .cake-debug divs via JS. Thats why it was duplicated.

@LordSimal
LordSimal force-pushed the 5.x-remove-deprecations branch from 51b4963 to 9a80b63 Compare May 5, 2023 11:34
@LordSimal
LordSimal merged commit ff083fb into 5.x May 5, 2023
@LordSimal
LordSimal deleted the 5.x-remove-deprecations branch May 5, 2023 11:36
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.

2 participants