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

Add option to disable the table header #53

Closed
vendeeglobe opened this issue Jul 22, 2020 · 20 comments
Closed

Add option to disable the table header #53

vendeeglobe opened this issue Jul 22, 2020 · 20 comments

Comments

@vendeeglobe
Copy link

Currently we patch php-diff class for Inline and SideBySide diff mode, because the application add its own header with additional metadata. It would be nice to disable the header for instance via via the $customOptions.

    <thead>
        <tr>
            <th colspan="2">{$this->options['title1']}</th>
            <th colspan="2">{$this->options['title2']}</th>
        </tr>
    </thead>

diff_without_header

@DigiLive
Copy link
Collaborator

This flexibility exists in the possibility to write your own renderer or to extend the included renderers by your own class which can override any of the original methods.
E.g.:

<?php

declare(strict_types=1);

use jblond\Diff\Renderer\Html\SideBySide;

class WackoWikiSideBySide extends SideBySide
{
    /**
     * Generates a string representation of the opening of a predefined table and its header with titles from options.
     *
     * @return string HTML code representation of a table's header.
     */
    public function generateDiffHeader(): string
    {
        return <<<HTML
<table class="Differences DifferencesSideBySide">
    <thead>
        <tr>
            <th colspan="4">This is my overridden header</th>
        </tr>
    </thead>
HTML;
    }

}
    //$renderer = new SideBySide();
    $renderer = new WackoWikiSideBySide();
    echo $diff->Render($renderer);

Currently the master branch includes an interface which can be implemented for writing your own renderer classes.

@JBlond
Copy link
Owner

JBlond commented Jul 22, 2020

I think we need to add those things to the documentation.

@DigiLive
Copy link
Collaborator

Yes... that's why I suggested to enable the Wiki for this repo.
Since the library is quite evolving now, there's need for a proper manual.
Makes it both easier for the user as for us.

@JBlond
Copy link
Owner

JBlond commented Jul 23, 2020

I enabled the wiki.

@JBlond
Copy link
Owner

JBlond commented Jul 23, 2020

After #54 is merged, we can publish a new version, so you @vendeeglobe have the option to use a customs override function and don't to patch this any longer.

@JBlond
Copy link
Owner

JBlond commented Jul 23, 2020

@vendeeglobe You can use v2.2.0
If you have another request let us know.

@JBlond JBlond closed this as completed Jul 23, 2020
@DigiLive
Copy link
Collaborator

I enabled the wiki.

I've cloned the wiki to my local machine... During next week, I will add content to it.

@vendeeglobe
Copy link
Author

This solutions would suffice at least for just turning off the header in HTML renderer, without excluding the possibility of having your own header.

jfcherng/php-diff@5f0dd2d

@JBlond
Copy link
Owner

JBlond commented Jun 4, 2021

I look at that.

@JBlond JBlond reopened this Jun 4, 2021
@DigiLive
Copy link
Collaborator

DigiLive commented Jun 4, 2021

Did you look at the wiki and discussions?
#96

All generated output can by modified as wished.

@JBlond
Copy link
Owner

JBlond commented Jun 4, 2021

Well I did. The one thing is an configuration option. The other one needs a bit more programming skills.

@vendeeglobe
Copy link
Author

Yes I have read the wiki and the discussion. For this special case I prefer a configuration option and not have to maintain additional code.

@JBlond
Copy link
Owner

JBlond commented Oct 26, 2021

@vendeeglobe If you eager you make create a pull request for that feature. I would be happy to merge it.

@vendeeglobe
Copy link
Author

I'll do my best.

@vendeeglobe
Copy link
Author

// Options for generating the diff.
$options = [
    'ignoreWhitespace' => true,
    'ignoreCase'       => true,
    'context'          => 2,
    'showHeader'       => 0,
];

// Initialize the diff class.
$diff = new Diff($sampleA, $sampleB , $options);
public function generateDiffHeader(): string
{
    $html =  <<<HTML
<table class="Differences DifferencesUnified">
HTML;

    if ($this->options['showHeader']) {
        $html .= <<<HTML
    <thead>
        <tr>
            <th>{$this->options['title1']}</th>
            <th>{$this->options['title2']}</th>
            <th>Differences</th>
        </tr>
    </thead>
HTML;
    }

    return $html;
}

Where should the default value be set? It seems that the option showHeader gets not merged in MainRendererAbstract class.

@JBlond
Copy link
Owner

JBlond commented Oct 27, 2021

The default options https://github.com/JBlond/php-diff/blob/master/lib/jblond/Diff.php#L60
I think showHeader should be default true
That is the behavior most people would expect and it is now.

@vendeeglobe
Copy link
Author

I did, but somehow the option, default or not, did not get passed on to work in the Html renderer.

@DigiLive
Copy link
Collaborator

Suggested option is a renderer specific option.
It shouldn't be included in the Diff class nor the MainRenderer classes.
It should be included in the renderer class itself, E.g. here:

private $subOptions = [
'format' => 'html',
'insertMarkers' => ['<ins>', '</ins>'],
'deleteMarkers' => ['<del>', '</del>'],
'title1' => 'Version1',
'title2' => 'Version2',
];

@JBlond Please read the email I've sent you about this issue.

@vendeeglobe
Copy link
Author

    private $subOptions = [
        'format'        => 'html',
        'insertMarkers' => ['<ins>', '</ins>'],
        'deleteMarkers' => ['<del>', '</del>'],
        'title1'        => 'Version1',
        'title2'        => 'Version2',
        'showHeader'    => true,
    ];

I did that, however still the default value gets not overwritten by the value provided with the $options.

// Initialize the diff class.
$diff = new Diff($sampleA, $sampleB , $options);

@DigiLive
Copy link
Collaborator

Like I said, it should be added to the options array of the subRenderer and therefor the value should be passed to the constructor of the subRenderer.

// Initialize the diff class.
$diff = new Diff($sampleA, $sampleB);

// Choose Renderer.
$renderer = new SideBySide(['showHeader' => true]);

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