Skip to content

Conversation

@annexus254
Copy link
Contributor

When loading the Database Context using the Context::load() method, throwing a LoaderException triggers most, if not all debuggers e.g. Xdebug. However, this is a false positive since the caller of the Context::load() method catches the exception thrown and takes the necessary action which is to attempt and load the next closest context.

Hence I think it is more apt for Context::load() method to return a boolean indicating a success or failure, in order to deal with this problem of debuggers being falsely triggered.

In case an error occurs during loading of a context, let the Context::load()
method return a boolean value indicating whether the context was successfully
loaded or not.
Throwing an Exception causes a false positive in debuggers,since the Context::load()
method does the correct thing anyway
In case an error occurs during loading of a context, let the Context::load()
method return a boolean value indicating whether the context was successfully
loaded or not.
Throwing an Exception causes a false positive in debuggers,since the Context::load()
method does the correct thing anyway
Resolve Merge Conflict caused by ./vendor/bin/phpcbf

if (isset(static::$KEYWORDS[$str])) {
if ($isReserved && ! (static::$KEYWORDS[$str] & Token::FLAG_KEYWORD_RESERVED)) {
if ($isReserved && !(static::$KEYWORDS[$str] & Token::FLAG_KEYWORD_RESERVED)) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ($isReserved && !(static::$KEYWORDS[$str] & Token::FLAG_KEYWORD_RESERVED)) {
if ($isReserved && ! (static::$KEYWORDS[$str] & Token::FLAG_KEYWORD_RESERVED)) {

public static function isOperator($str)
{
if (! isset(static::$OPERATORS[$str])) {
if (!isset(static::$OPERATORS[$str])) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!isset(static::$OPERATORS[$str])) {
if (! isset(static::$OPERATORS[$str])) {

}

if ((static::$MODE & self::SQL_MODE_NO_ENCLOSING_QUOTES) && (! static::isKeyword($str, true))) {
if ((static::$MODE & self::SQL_MODE_NO_ENCLOSING_QUOTES) && (!static::isKeyword($str, true))) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ((static::$MODE & self::SQL_MODE_NO_ENCLOSING_QUOTES) && (!static::isKeyword($str, true))) {
if ((static::$MODE & self::SQL_MODE_NO_ENCLOSING_QUOTES) && (! static::isKeyword($str, true))) {

Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

This looks good, can you squash the commits and also delete the LoaderException class file ?

@williamdes williamdes changed the base branch from master to 6.0.x August 8, 2022 10:41
@williamdes williamdes changed the base branch from 6.0.x to master August 8, 2022 10:42
@williamdes
Copy link
Member

By the way because this PR is open using your master branch it will cause some issues that I will solve before merging into 6.0.x :)
The best is to create a new branch based onto the base branch you want to merge into

@annexus254
Copy link
Contributor Author

This looks good, can you squash the commits and also delete the LoaderException class file ?

Sure, the important commits are now present on the exceptionRemoval branch

By the way because this PR is open using your master branch it will cause some issues that I will solve before merging into 6.0.x :) The best is to create a new branch based onto the base branch you want to merge into

Done. The new branch is exceptionRemoval and is based off of the main branch.

@williamdes
Copy link
Member

This looks good, can you squash the commits and also delete the LoaderException class file ?

Sure, the important commits are now present on the exceptionRemoval branch

By the way because this PR is open using your master branch it will cause some issues that I will solve before merging into 6.0.x :) The best is to create a new branch based onto the base branch you want to merge into

Done. The new branch is exceptionRemoval and is based off of the main branch.

Thanks !

@williamdes
Copy link
Member

Duplicate of #384

@williamdes williamdes marked this as a duplicate of #384 Aug 8, 2022
@williamdes williamdes closed this Aug 8, 2022
@williamdes williamdes self-assigned this Aug 8, 2022
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