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

First char is omitted in result #51

Closed
bAngerman opened this issue Feb 3, 2022 · 13 comments
Closed

First char is omitted in result #51

bAngerman opened this issue Feb 3, 2022 · 13 comments
Labels
bug Something isn't working upstream Upstream issue.

Comments

@bAngerman
Copy link

bAngerman commented Feb 3, 2022

I am using version 6.11.1
I am encountering an issue where only the first character is missing when a diff is calculated.

Here is my function containing use of the php-diff tool.

public function getContentDiff($old, $new)
{
    $renderer_name = 'Combined';
    $differ_options = [];
    $renderer_options = [
        'detailLevel' => 'word',
    ];

    return DiffHelper::calculate($old, $new, $renderer_name, $differ_options, $renderer_options);
}

As you can see from the following image, after an edit is made the diff omits the first character of the line in the diff.

Screen Shot 2022-02-03 at 3 55 07 PM

Any advice?

@jfcherng
Copy link
Owner

jfcherng commented Feb 4, 2022

Any advice?

Provide a reproduce-able test case. I.e., $old and $new.

@jfcherng jfcherng added the need more info Further information is needed label Feb 4, 2022
@bAngerman
Copy link
Author

public function getContentDiff($old, $new)
{
    $renderer_name = 'Combined';
    $differ_options = [];
    $renderer_options = [
        'detailLevel' => 'word',
    ];

    $diff = DiffHelper::calculate($old, $new, $renderer_name, $differ_options, $renderer_options);

    dd( $old, $new, $diff );
    return $diff;
}

Yields the following output:

"This is a brand new note! New text."

"Modified the content."

"<table class="diff-wrapper diff diff-html diff-combined"><thead><tr><th>Differences</th></tr></thead><tbody class="change change-rep"><tr data-type="-"><td class="old"> <del>his is a brand new note! New text</del>.</td></tr><tr data-type="+"><td class="new"> <ins>odified the content</ins>.</td></tr></tbody></table>[ ◀]()"

As you can see, $diff omits the first character of the line for deleted, and inserted lines.

@jfcherng
Copy link
Owner

jfcherng commented Feb 4, 2022

Unfortunately, I still can't reproduce this with your given information with my example demo.

image

Can you provide a self-contained repository so that I can just clone and run for reproduction?


I cannot reproduce it with your codes as well.

152581821-c3763bc4-c891-49a0-b317-2002e55269ea

@fomk
Copy link

fomk commented Feb 8, 2022

It's not missing it is replaced with space wich comes from here, if you change it to any other char it will prepend it. It does not reproduces at least on php 8.0, but reproduces on php 7.2.

It is not package issue, it seems like there is no header passed on php7.x so php-mb-string trims first character and prepend space to string.
simple test file:
<?php $str = 'Some text'; $header = \substr(\iconv('UTF-8', 'UTF-32', ' '), 0, 4); $str = \substr(\iconv('UTF-8', 'UTF-32', $str), 4); $str = \iconv('UTF-32', 'UTF-8', $header.$str); echo $str;

@fomk
Copy link

fomk commented Feb 8, 2022

It seems like alpine iconv issue, can reproduce on all php:x.x-fpm-alpine docker images, works fine on debian based images.

@jfcherng
Copy link
Owner

jfcherng commented Feb 9, 2022

It looks like many people have reported that alpine's iconv has some issues... with various solutions I didn't try.

Could you test whether this branch workarounds the issue?

@fomk
Copy link

fomk commented Feb 20, 2022

php:7.4-fpm-alpine now it append A 2 times:

include __DIR__ . '/vendor/autoload.php';

use Jfcherng\Diff\DiffHelper;

echo DiffHelper::calculate("Test", "TestTest", 'Inline', [], ['language' => 'rus','lineNumbers' => false, 'detailLevel' => 'word', 'showHeader' => false, 'separateBlock' => true]);

Output:

<td class="old">A<del>ATest</del></td>
<td class="new">A<ins>ATestTest</ins></td>

@jfcherng jfcherng added bug Something isn't working and removed need more info Further information is needed labels Feb 21, 2022
@jfcherng
Copy link
Owner

jfcherng commented Feb 21, 2022

@bAngerman I pulled php:7.4-fpm-alpine image and did some tests. I think this has been fixed in 6.11.2, which is just released. If it works for you, please close this issue. Thanks 😄

@jfcherng jfcherng added the upstream Upstream issue. label Feb 21, 2022
@bAngerman
Copy link
Author

bAngerman commented Feb 28, 2022

I have updated my version to your provided version, but issues still remain.
Using the same pattern of dd( $old, $new, $diff ); the output has become:

"original message"

"Additional content original message"

"<table class="diff-wrapper diff diff-html diff-combined"><thead><tr><th>Differences</th></tr></thead><tbody class="change change-rep"><tr data-type="!"><td class="rep"> <del>riginal</del><ins>dditional content original</ins> message</td></tr></tbody></table>"

@jfcherng
Copy link
Owner

jfcherng commented Mar 1, 2022

@bAngerman Are you using a PHP docker image or something that I can use for reproducing the issue?

@bAngerman
Copy link
Author

FROM php:7.4-fpm-alpine

# install the PHP extensions we need
RUN apk update \
#    && apk --no-cache add libonig-dev \
    && apk --no-cache add freetype-dev jpeg-dev libpng-dev libjpeg-turbo-dev libmcrypt-dev libxml2-dev autoconf g++ imagemagick imagemagick-dev libtool make pcre-dev zip unzip icu-dev icu-libs php7-intl \
    && /bin/ln -s /usr/bin/zip   /usr/local/bin/zip   \
    && /bin/ln -s /usr/bin/unzip /usr/local/bin/unzip \
    && docker-php-ext-configure gd --with-jpeg \
    && docker-php-ext-install -j$(nproc) gd \
    && docker-php-ext-install mysqli opcache soap iconv pdo_mysql exif \
    && docker-php-ext-configure intl \
    && docker-php-ext-install intl \
    && docker-php-ext-enable intl \
    && pecl install imagick && docker-php-ext-enable imagick \

    # xdebug on docker slow things down
    # && pecl install xdebug  && docker-php-ext-enable xdebug \

    # save up some megabytes for generated docker images: 428MB -> 214MB
    #
    && apk del autoconf file g++ libtool gcc freetype-dev libjpeg-turbo-dev libxml2-dev libc-dev isl libatomic make re2c mpc1 mpfr4 musl-dev icu-dev build-base linux-headers pcre-dev openssl-dev libxml2-dev imagemagick-dev \
    && sleep 5 \
    && rm -rf /tmp/* /usr/share/man /usr/src/* /var/cache

is the Dockerfile I am using for PHP

@jfcherng
Copy link
Owner

jfcherng commented Mar 1, 2022

@bAngerman Unfortunately, I can't reproduce this with your Dockerfile and proposed codes.

<?php

declare(strict_types=1);

require __DIR__ . '/vendor/autoload.php';

use Jfcherng\Diff\DiffHelper;

function getContentDiff($old, $new)
{
    $renderer_name = 'Combined';
    $differ_options = [];
    $renderer_options = [
        'detailLevel' => 'word',
    ];

    $diff = DiffHelper::calculate($old, $new, $renderer_name, $differ_options, $renderer_options);

    return $diff;
}

echo "\n\n";
echo getContentDiff(
    'This is a brand new note! New text.',
    'Modified the content.',
);
echo "\n\n";

Outputs:

/var/www/html # php index.php
PHP Warning:  PHP Startup: Unable to load dynamic library 'imagick' (tried: /usr/local/lib/php/extensions/no-debug-non-zts-20190902/imagick (Error loading shared library /usr/local/lib/php/extensions/no-debug-non-zts-20190902/imagick: No such file or directory), /usr/local/lib/php/extensions/no-debug-non-zts-20190902/imagick.so (Error loading shared library libgomp.so.1: No such file or directory (needed by /usr/local/lib/php/extensions/no-debug-non-zts-20190902/imagick.so))) in Unknown on line 0


<table class="diff-wrapper diff diff-html diff-combined"><thead><tr><th>Differences</th></tr></thead><tbody class="change change-rep"><tr data-type="-"><td class="old"><del>This is a brand new note! New text</del>.</td></tr><tr data-type="+"><td class="new"><ins>Modified the content</ins>.</td></tr></tbody></table>

@fomk Can you still reproduce this after 6.11.2?

@bAngerman
Copy link
Author

My mistake @jfcherng , I improperly installed the version. It is now working that I have resolved my composer issues.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream Upstream issue.
Projects
None yet
Development

No branches or pull requests

3 participants