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
Fix type check that caused empty sources #273
Fix type check that caused empty sources #273
Conversation
@CodiumAI-Agent /improve --extended |
if ( ( ! $image_uses_standard_source && $source === '' ) || ( $image_uses_standard_source && $exclude_standard ) ) { | ||
if ( $image_uses_standard_source && $exclude_standard ) { | ||
ISC_Log::log( sprintf( 'image %d: "own" sources are excluded', $attachment_id ) ); | ||
} else { | ||
ISC_Log::log( sprintf( 'image %d: skipped because of empty source', $attachment_id ) ); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
if ( Standard_Source::standard_source_is( 'exclude' ) && $connected_atts[ $_attachment->ID ]['standard'] ) { | ||
unset( $connected_atts[ $_attachment->ID ] ); | ||
continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The variable $_attachment
in line 647 is not very descriptive. Consider renaming it to something more meaningful. [best practice]
if ( Standard_Source::standard_source_is( 'exclude' ) && $connected_atts[ $_attachment->ID ]['standard'] ) { | |
unset( $connected_atts[ $_attachment->ID ] ); | |
continue; | |
if ( Standard_Source::standard_source_is( 'exclude' ) && $connected_atts[ $attachment_id ]['standard'] ) { | |
unset( $connected_atts[ $attachment_id ] ); | |
continue; | |
} |
if ( ( ! $image_uses_standard_source && $source === '' ) || ( $image_uses_standard_source && $exclude_standard ) ) { | ||
if ( $image_uses_standard_source && $exclude_standard ) { | ||
ISC_Log::log( sprintf( 'image %d: "own" sources are excluded', $attachment_id ) ); | ||
} else { | ||
ISC_Log::log( sprintf( 'image %d: skipped because of empty source', $attachment_id ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Simplify the conditional logic in the if
statement at line 479. You can combine the two conditions into one by using the logical OR operator (||
). [maintainability]
if ( ( ! $image_uses_standard_source && $source === '' ) || ( $image_uses_standard_source && $exclude_standard ) ) { | |
if ( $image_uses_standard_source && $exclude_standard ) { | |
ISC_Log::log( sprintf( 'image %d: "own" sources are excluded', $attachment_id ) ); | |
} else { | |
ISC_Log::log( sprintf( 'image %d: skipped because of empty source', $attachment_id ) ); | |
if ( ! $image_uses_standard_source && $source === '' || $image_uses_standard_source && $exclude_standard ) { | |
if ( $image_uses_standard_source && $exclude_standard ) { | |
ISC_Log::log( sprintf( 'image %d: "own" sources are excluded', $attachment_id ) ); | |
} else { | |
ISC_Log::log( sprintf( 'image %d: skipped because of empty source', $attachment_id ) ); | |
} | |
} |
… if the option to exclude standard sources from sources lists was selected.
9df6a4b
to
d157e72
Compare
A combination of a hardened type check, image sources did not show up if the option to exclude standard sources from sources lists was selected.