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

langs #91

Closed
wants to merge 3 commits into from
Closed

langs #91

wants to merge 3 commits into from

Conversation

N6REJ
Copy link
Contributor

@N6REJ N6REJ commented Oct 27, 2024

User description

mixture of fixes. Somehow left over and never merged.
lang: Fixes truncated words in about box
copygright fixes
unknown line seperator changes


PR Type

Enhancement, Bug fix


Description

  • Added a new section for downloading "Bruno" with version display in the homepage summary.
  • Fixed method call from setSmtpPort to setPort in class.bin.xlight.php.
  • Reformatted file header comments for consistency across multiple classes.
  • Updated copyright comment format.
  • Replaced entire content of the PWTech.ini file with new settings.

Changes walkthrough 📝

Relevant files
Enhancement
1 files
hp.summary.html
Add download section for Bruno in homepage summary.           

core/resources/homepage/tpls/hp.summary.html

  • Added a new section for downloading "Bruno" with version display.
  • Updated copyright comment format.
  • +16/-6   
    Formatting
    6 files
    class.bin.apache.php
    Reformat file header comment in BinApache class.                 

    core/classes/bins/class.bin.apache.php

    • Reformatted the file header comment for consistency.
    +1069/-1068
    class.action.editVhost.php
    Reformat file header comment in ActionEditVhost class.     

    core/classes/actions/class.action.editVhost.php

    • Reformatted the file header comment for consistency.
    +189/-188
    class.tool.python.php
    Reformat file header comment in ToolPython class.               

    core/classes/tools/class.tool.python.php

    • Reformatted the file header comment for consistency.
    +6/-20   
    class.tpl.app.rebuildIni.php
    Reformat file header comment in TplAppRebuildIni class.   

    core/classes/tpls/app/class.tpl.app.rebuildIni.php

    • Reformatted the file header comment for consistency.
    +40/-39 
    class.homepage.php
    Reformat file header comment in Homepage class.                   

    core/classes/class.homepage.php

    • Reformatted the file header comment for consistency.
    +6/-13   
    class.tpl.app.python.php
    Reformat file header comment in TplAppPython class.           

    core/classes/tpls/app/class.tpl.app.python.php

    • Reformatted the file header comment for consistency.
    +6/-12   
    Bug fix
    1 files
    class.bin.xlight.php
    Update copyright format and method call in PHP class         

    core/classes/bins/class.bin.xlight.php

  • Fixed copyright comment formatting.
  • Changed method call from setSmtpPort to setPort.
  • +7/-6     
    Configuration changes
    1 files
    PWTech.ini
    Update configuration settings in PWTech.ini file                 

    core/libs/pwgen/Data/settings/PWTech.ini

    • Replaced entire content of the INI file with new settings.
    +183/-183

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Method Name Change
    The method name was changed from setSmtpPort to setPort. This change should be validated to ensure it doesn't break existing functionality or cause issues elsewhere in the codebase.

    Configuration File Overwrite
    The entire content of the PWTech.ini file has been replaced. This could potentially overwrite important settings. The changes should be carefully reviewed to ensure no critical configurations are lost.

    @N6REJ N6REJ closed this Oct 27, 2024
    @N6REJ N6REJ deleted the langs branch October 27, 2024 11:17
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Correct method name to ensure consistency and prevent potential errors

    Replace setSmtpPort with setPort to match the method name used in the parent class
    or other related classes. This ensures consistency across the codebase and prevents
    potential errors.

    core/classes/bins/class.bin.xlight.php [182]

    -$this->setSmtpPort($port);
    +$this->setPort($port);
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The suggestion correctly identifies a method name inconsistency, replacing setSmtpPort with setPort, which aligns with the changes made in the PR. This correction is crucial for maintaining consistency across the codebase and preventing potential errors due to incorrect method calls.

    9
    Enhancement
    Improve string concatenation using modern PHP syntax for better readability and performance

    Consider using a more modern PHP syntax for string concatenation. Instead of using
    the dot (.) operator, you can use string interpolation with curly braces for better
    readability and performance.

    core/classes/bins/class.bin.apache.php [713-719]

    -$result .= 'Alias /' . $name . ' "' . $dest . '"' . PHP_EOL . PHP_EOL .
    -    '<Directory "' . $dest . '">' . PHP_EOL .
    -    '    Options Indexes FollowSymLinks MultiViews' . PHP_EOL .
    -    '    AllowOverride all' . PHP_EOL .
    -    $this->getRequiredContent() . PHP_EOL .
    -    '</Directory>' . PHP_EOL;
    +$result .= <<<EOT
    +Alias /{$name} "{$dest}"
     
    +<Directory "{$dest}">
    +    Options Indexes FollowSymLinks MultiViews
    +    AllowOverride all
    +{$this->getRequiredContent()}
    +</Directory>
    +EOT;
    +
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion to use PHP's heredoc syntax for string concatenation improves readability and performance. It is a modern approach that simplifies the code and reduces the risk of errors in string manipulation.

    8
    Improve HTML semantics and consistency in the download section

    Consider using a more semantic HTML structure for the Bruno download section. Use a
    list item (

  • ) instead of a for consistency with other download items.

    core/resources/homepage/tpls/hp.summary.html [103-111]

    -<span class = "list-group-item">
    -    <a aria-label = "Download Bruno" href = "<?php echo Util::getWebsiteUrl('module/bruno', '#releases'); ?>" target = "_blank"
    -       title = "<?php echo $downloadTitle; ?>">
    -        <span class = "float-end ms-2"><i class = "fa-solid fa-cloud-arrow-down"></i>
    -        </span>
    +<li class="list-group-item">
    +    <a aria-label="Download Bruno" href="<?php echo Util::getWebsiteUrl('module/bruno', '#releases'); ?>" target="_blank"
    +       title="<?php echo $downloadTitle; ?>">
    +        <span class="float-end ms-2"><i class="fa-solid fa-cloud-arrow-down"></i></span>
         </a>
    -    <span class = "float-end badge text-bg-primary"><?php echo $bearsamppTools->getBruno()->getVersion(); ?></span>
    -    <span><?php echo $bearsamppLang->getValue(Lang::BRUNO); ?></span>
    -</span>
    +    <span class="float-end badge text-bg-primary"><?php echo $bearsamppTools->getBruno()->getVersion(); ?></span>
    +    <?php echo $bearsamppLang->getValue(Lang::BRUNO); ?>
    +</li>
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion to use a

  • element instead of a for the list item improves HTML semantics and consistency, aligning with best practices for list structures. This enhances readability and maintainability of the code.

  • 7
    Performance
    Improve file processing efficiency by reading and processing the file line by line

    Consider using a more efficient approach for file reading. Instead of using
    file_get_contents() followed by preg_replace(), you can use file() to read the file
    line by line and process it more efficiently, especially for large files.

    core/classes/bins/class.bin.apache.php [785-797]

    -$conf = file_get_contents($this->getConf());
    -Util::logTrace('refreshConf ' . $this->getConf());
    -preg_match('/' . self::TAG_START_SWITCHONLINE . '(.*?)' . self::TAG_END_SWITCHONLINE . '/s', $conf, $matches);
    -Util::logTrace(isset($matches[1]) ? print_r($matches[1], true) : 'N/A');
    +$lines = file($this->getConf());
    +$newContent = '';
    +$inSwitchBlock = false;
    +$count = 0;
     
    -if ($putOnline) {
    -    $conf = preg_replace('/' . self::TAG_START_SWITCHONLINE . '(.*?)' . self::TAG_END_SWITCHONLINE . '/s', $onlineContent, $conf, -1, $count);
    -} else {
    -    $conf = preg_replace('/' . self::TAG_START_SWITCHONLINE . '(.*?)' . self::TAG_END_SWITCHONLINE . '/s', $offlineContent, $conf, -1, $count);
    +foreach ($lines as $line) {
    +    if (strpos($line, self::TAG_START_SWITCHONLINE) !== false) {
    +        $inSwitchBlock = true;
    +        $newContent .= $putOnline ? $onlineContent : $offlineContent;
    +        $count++;
    +    } elseif (strpos($line, self::TAG_END_SWITCHONLINE) !== false) {
    +        $inSwitchBlock = false;
    +    } elseif (!$inSwitchBlock) {
    +        $newContent .= $line;
    +    }
     }
    -file_put_contents($this->getConf(), $conf);
     
    +file_put_contents($this->getConf(), $newContent);
    +
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: The suggestion to read and process the file line by line can enhance performance, especially for large files. However, the complexity of the change might not significantly impact performance in all scenarios, hence a moderate score.

    6

    💡 Need additional feedback ? start a PR chat

    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.

    1 participant