Skip to content
This repository has been archived by the owner on Sep 20, 2021. It is now read-only.

Update Llk.php #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update Llk.php #91

wants to merge 1 commit into from

Conversation

flip111
Copy link

@flip111 flip111 commented Oct 21, 2018

Fix bug #90

@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling 095c4f4 on flip111:patch-2 into c620f44 on hoaproject:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling 095c4f4 on flip111:patch-2 into c620f44 on hoaproject:master.

@@ -125,7 +125,7 @@ public static function save(Parser $parser, $className)
' \'' . $tokenName . '\' => \'' .
str_replace(
['\'', '\\\\'],
['\\\'', '\\\\\\'],
['\\\'', '\\\\\\\\'],
Copy link

Choose a reason for hiding this comment

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

It seems like dumping of these values must be done via var_export(..., true). Right now it's unsafe and unpredictable.

Copy link
Author

Choose a reason for hiding this comment

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

let's fix the bug within the current mechanism first because it's an easy merge. You can open a PR where var_export is used

Copy link
Member

Choose a reason for hiding this comment

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

Actually, using var_export is not a bad idea at all. And it will simplify the code. Are you willing to try to update with var_export please?

@@ -125,7 +125,7 @@ public static function save(Parser $parser, $className)
' \'' . $tokenName . '\' => \'' .
str_replace(
['\'', '\\\\'],
['\\\'', '\\\\\\'],
['\\\'', '\\\\\\\\'],
Copy link
Member

Choose a reason for hiding this comment

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

Actually, using var_export is not a bad idea at all. And it will simplify the code. Are you willing to try to update with var_export please?

@unkind
Copy link

unkind commented Dec 1, 2018

I could help with this PR, but unfortunately I face with such invalid types too often:
image

I suggest to replace Consistency::flexEntity magic with native class_alias().

@Hywan
Copy link
Member

Hywan commented Dec 3, 2018

@unkind Check this command https://github.com/hoaproject/Devtools#expandflexentities to help you.

@unkind
Copy link

unkind commented Dec 3, 2018

@Hywan I have to find out the real purpose over class_alias() first :) #95 (comment)
I can make PR for replacing it.

@Hywan
Copy link
Member

Hywan commented Dec 4, 2018

@unkind You can use devtools:expandflexentities to solve your issue with your IDE :-). We don't have plan yet to remove flex entities for BC reasons.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

4 participants