Skip to content

Fix PHP indent to tabs - #528

Open
thinklanguages wants to merge 1 commit into
mainfrom
yd8bkb-codex/convert-indentation-to-tabs-in-php-files
Open

Fix PHP indent to tabs#528
thinklanguages wants to merge 1 commit into
mainfrom
yd8bkb-codex/convert-indentation-to-tabs-in-php-files

Conversation

@thinklanguages

@thinklanguages thinklanguages commented Jun 28, 2025

Copy link
Copy Markdown
Owner

Summary

  • enforce tabs for all PHP indentation
  • clean up admin and dashboard code

Testing

  • composer lint --working-dir=nuclear-engagement (fails: composer not found)

https://chatgpt.com/codex/tasks/task_e_685f7978df7c83278d6890ef69741860

Description by Korbit AI

What change is being made?

Convert all PHP files from using spaces for indentation to using tabs across the nuclear-engagement codebase.

Why are these changes being made?

The change to tab indentation is for consistency with the project's coding standards, which mandate the use of tabs. This aligns the code structure properly, making it easier to read and maintain for all developers working with the nuclear-engagement plugin.

Is this description stale? Ask me to generate a new description by commenting /korbit-generate-pr-description

@korbit-ai korbit-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Readability Incorrectly indented file-level DocBlock ▹ view
Documentation Incorrect File Header Format ▹ view
Readability Fix indentation alignment ▹ view
Documentation Incomplete loader purpose documentation ▹ view
Design Poor Template Encapsulation ▹ view
Documentation Inconsistent docblock formatting and insufficient module description ▹ view
Readability Unclear Object Instantiation ▹ view
Design Direct dependency management in loader ▹ view
Documentation Insufficient legacy code documentation ▹ view
Performance Avoidable Database Query on Every Save ▹ view
Files scanned
File Path Reviewed
nuclear-engagement/templates/admin/notice.php
nuclear-engagement/inc/Services/AdminNoticeService.php
nuclear-engagement/inc/Traits/SettingsPersistenceTrait.php
nuclear-engagement/inc/Modules/Summary/loader.php
nuclear-engagement/inc/Modules/Quiz/loader.php
nuclear-engagement/inc/Modules/TOC/Nuclen_TOC_Headings.php
nuclear-engagement/admin/Admin.php
nuclear-engagement/inc/Utils/Utils.php
nuclear-engagement/admin/Traits/AdminMenu.php
nuclear-engagement/inc/Core/MetaRegistration.php
nuclear-engagement/front/Controller/Rest/ContentController.php
nuclear-engagement/inc/Modules/TOC/Nuclen_TOC_Utils.php
nuclear-engagement/inc/Core/ContainerRegistrar.php
nuclear-engagement/inc/Services/LoggingService.php
nuclear-engagement/admin/Dashboard.php

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

public static function register_meta_keys(): void {
// Get allowed post types from settings
$post_types = SettingsFunctions::get_array( 'generation_post_types', array( 'post' ) );
$post_types = SettingsFunctions::get_array( 'generation_post_types', array( 'post' ) );

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fix indentation alignment category Readability

Tell me more
What is the issue?

Inconsistent indentation using excessive spaces/tabs compared to surrounding code.

Why this matters

Inconsistent indentation makes code harder to scan visually and affects code readability in different editors.

Suggested change ∙ Feature Preview
        $post_types = SettingsFunctions::get_array('generation_post_types', array('post'));
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines 3 to +7
/**
* File: includes/ContainerRegistrar.php
*
* Registers services and controllers in the DI container.
*/
* File: includes/ContainerRegistrar.php
*
* Registers services and controllers in the DI container.
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incorrect File Header Format category Documentation

Tell me more
What is the issue?

The file header documentation is incorrectly indented with tabs and contains redundant file path information.

Why this matters

Inconsistent indentation makes documentation harder to read in IDEs and the redundant file path adds no value since it's already clear from the file's location.

Suggested change ∙ Feature Preview

/**

  • Container registration service.
  • Handles registration of services and controllers in the dependency injection container.
    */
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines 2 to +8
/**
* File: modules/quiz/loader.php
*
* Loads the Nuclen Quiz module.
*
* @package NuclearEngagement
*/
* File: modules/quiz/loader.php
*
* Loads the Nuclen Quiz module.
*
* @package NuclearEngagement
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incomplete loader purpose documentation category Documentation

Tell me more
What is the issue?

The file docblock doesn't explain the purpose and responsibilities of this loader beyond stating it 'loads' the module.

Why this matters

Without understanding the loader's specific role and responsibilities, other developers may misuse or modify it incorrectly.

Suggested change ∙ Feature Preview

/**

  • File: modules/quiz/loader.php
  • Bootstraps the Quiz module by initializing dependencies, loading required files,
  • and setting up the appropriate admin or frontend handlers.
  • @Package NuclearEngagement
    */
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines 3 to +5
/**
* Handles admin notices for the plugin.
*/
* Handles admin notices for the plugin.
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incorrectly indented file-level DocBlock category Readability

Tell me more
What is the issue?

File-level DocBlock is incorrectly indented with tabs, breaking from PSR standards where file-level documentation should be at the root level without indentation.

Why this matters

Inconsistent documentation formatting can make it harder to read the code's documentation and breaks from established PHP documentation standards.

Suggested change ∙ Feature Preview
/**
 * Handles admin notices for the plugin.
 */
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines 6 to +13
/**
* Single admin notice.
*
* Variables:
* - $msg (string) Notice message.
*
* @package NuclearEngagement\Admin
*/
* Single admin notice.
*
* Variables:
* - $msg (string) Notice message.
*
* @package NuclearEngagement\Admin
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Poor Template Encapsulation category Design

Tell me more
What is the issue?

The template lacks proper encapsulation by directly accessing variables without a clear interface or data validation.

Why this matters

Direct variable access without proper validation or a clear interface makes the template fragile and harder to maintain, violating the Single Responsibility Principle.

Suggested change ∙ Feature Preview

Create a Notice class or interface to handle notice data and validation:

class AdminNotice {
    private string $message;
    private string $type;
    
    public function __construct(string $message, string $type = 'error') {
        $this->message = $message;
        $this->type = $type;
    }
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines 2 to +8
/**
* File: modules/summary/loader.php
*
* Loads the Nuclen Summary sub-module.
*
* @package NuclearEngagement
*/
* File: modules/summary/loader.php
*
* Loads the Nuclen Summary sub-module.
*
* @package NuclearEngagement
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Inconsistent docblock formatting and insufficient module description category Documentation

Tell me more
What is the issue?

The file docblock has inconsistent indentation with tabs, deviating from PSR documentation standards, and lacks meaningful context about the module's purpose.

Why this matters

Inconsistent documentation formatting reduces readability and maintainability. The description should explain the module's purpose, not just state what the file does.

Suggested change ∙ Feature Preview

/**

  • File: modules/summary/loader.php
  • Bootstrap file for the Summary module which handles content summarization
  • and display functionality within the Nuclear Engagement plugin.
  • @Package NuclearEngagement
    */
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines 47 to 50
new Nuclen_Summary_Shortcode( $settings, $front );
if ( is_admin() ) {
new Nuclen_Summary_Metabox( $settings );
new Nuclen_Summary_Metabox( $settings );
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unclear Object Instantiation category Readability

Tell me more
What is the issue?

Instantiating classes directly without assigning to variables makes the code's intent less clear and harder to follow.

Why this matters

Immediate object instantiation without assignment can make debugging more difficult and obscures the purpose of these objects in the codebase.

Suggested change ∙ Feature Preview

Assign instantiated objects to descriptively named variables:

$summary_shortcode = new Nuclen_Summary_Shortcode( $settings, $front );
if ( is_admin() ) {
	$summary_metabox = new Nuclen_Summary_Metabox( $settings );
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines 40 to 50
$settings = \NuclearEngagement\Core\SettingsRepository::get_instance();
$front = new \NuclearEngagement\Front\FrontClass(
'nuclear-engagement',
defined( 'NUCLEN_PLUGIN_VERSION' ) ? NUCLEN_PLUGIN_VERSION : '1.0.0',
$settings,
new \NuclearEngagement\Core\Container()
'nuclear-engagement',
defined( 'NUCLEN_PLUGIN_VERSION' ) ? NUCLEN_PLUGIN_VERSION : '1.0.0',
$settings,
new \NuclearEngagement\Core\Container()
);
new Nuclen_Summary_Shortcode( $settings, $front );
if ( is_admin() ) {
new Nuclen_Summary_Metabox( $settings );
new Nuclen_Summary_Metabox( $settings );
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Direct dependency management in loader category Design

Tell me more
What is the issue?

The loader file is directly instantiating and managing dependencies instead of using a proper dependency injection container or service provider pattern.

Why this matters

This approach creates tight coupling between components and makes the code harder to test and maintain. It also violates the Dependency Inversion Principle of SOLID.

Suggested change ∙ Feature Preview
// Create a service provider class
class SummaryServiceProvider implements ServiceProviderInterface {
    public function register(Container $container) {
        $container->singleton('settings', function() {
            return SettingsRepository::get_instance();
        });
        
        $container->singleton('front', function($container) {
            return new FrontClass(
                'nuclear-engagement',
                defined('NUCLEN_PLUGIN_VERSION') ? NUCLEN_PLUGIN_VERSION : '1.0.0',
                $container->get('settings'),
                $container
            );
        });
        
        $container->bind('summary.shortcode', function($container) {
            return new Nuclen_Summary_Shortcode(
                $container->get('settings'),
                $container->get('front')
            );
        });
    }
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

}

// Also update legacy option for backward compatibility.
// Also update legacy option for backward compatibility.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Insufficient legacy code documentation category Documentation

Tell me more
What is the issue?

The inline comment doesn't explain why backward compatibility is needed or until when it should be maintained.

Why this matters

Future developers won't know when this legacy code can be safely removed, potentially leading to unnecessary code maintenance.

Suggested change ∙ Feature Preview

// Update legacy 'nuclear_engagement_setup' option for backward compatibility with v0.x
// TODO: Remove after version 2.0.0 when all users have migrated to new format

Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.


// Also update legacy option for backward compatibility.
// Also update legacy option for backward compatibility.
if ( $result && false !== get_option( 'nuclear_engagement_setup' ) ) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoidable Database Query on Every Save category Performance

Tell me more
What is the issue?

Unnecessary database query to get_option() performed on every settings save, regardless of whether legacy support is needed.

Why this matters

Each call to get_option() triggers a database query which adds unnecessary I/O overhead when legacy support might not be needed.

Suggested change ∙ Feature Preview

Cache or configure the need for legacy support during initialization rather than checking on every save. For example:

private bool $has_legacy_setup;

public function __construct() {
    $this->has_legacy_setup = false !== get_option('nuclear_engagement_setup');
}

// Then in save():
if ($result && $this->has_legacy_setup) {
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant