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

% progressbar #80

Merged
merged 5 commits into from
Aug 29, 2024
Merged

% progressbar #80

merged 5 commits into from
Aug 29, 2024

Conversation

N6REJ
Copy link
Contributor

@N6REJ N6REJ commented Aug 27, 2024

PR Type

enhancement, bug_fix


Description

  • Refactored the fetchAndUnzipModule method in class.action.quickPick.php to improve logging and progress handling, now reporting progress as a percentage.
  • Enhanced the unzipFile method in class.core.php for more accurate progress reporting, including handling for 100% completion.
  • Updated quickpick.js to improve progress bar handling and removed unnecessary aria-valuenow attribute setting.
  • Added copyright information to quickpick.js.

Changes walkthrough 📝

Relevant files
Enhancement
class.action.quickPick.php
Refactor fetchAndUnzipModule for better progress handling

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

  • Refactored fetchAndUnzipModule method for improved logging and
    progress handling.
  • Changed progress reporting to use percentage instead of file count.
  • Improved error handling for unsupported file extensions.
  • +52/-54 
    class.core.php
    Improve unzipFile method for accurate progress reporting 

    core/classes/class.core.php

  • Refactored unzipFile method to improve progress reporting.
  • Changed logging from debug to trace for detailed progress tracking.
  • Added handling for 100% progress reporting.
  • +73/-30 
    quickpick.js
    Update progress bar handling and add copyright                     

    core/resources/homepage/js/quickpick.js

  • Added copyright information.
  • Updated progress bar handling to improve clarity.
  • Removed setting of aria-valuenow attribute.
  • +14/-7   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    N6REJ added 5 commits August 24, 2024 08:37
    …ule` method in `class.action.quickPick.php` for improved logging and progress handling.
    
    Progressbar now shows % extracted properly
    … handling and add copyright information in `quickpick.js`
    
    uses 131mb of ram during processing
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Code Duplication
    The code for flushing output buffer is repeated multiple times. Consider extracting this into a separate function to reduce duplication.

    Performance Concern
    The unzipFile method reads the process output in small chunks and processes each line individually, which could be inefficient for large files. Consider buffering larger chunks of output before processing.

    Potential Bug
    The 'isCompleted' flag is checked after the loop, but it's not clear where it's set. This could lead to unexpected behavior if the flag is not properly managed.

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Add a timeout mechanism to the file extraction loop

    Consider adding a timeout mechanism to the file extraction loop to prevent potential
    infinite loops in case of unexpected behavior.

    core/classes/class.core.php [511-517]

    +$startTime = time();
    +$timeout = 300; // 5 minutes timeout
     while ( !feof( $process ) ) {
    +    if (time() - $startTime > $timeout) {
    +        Util::logError("Extraction process timed out after $timeout seconds");
    +        break;
    +    }
         $buffer .= fread( $process, 8192 ); // Read in chunks of 8KB
         while ( ($pos = strpos( $buffer, "\r" )) !== false ) {
             $line   = substr( $buffer, 0, $pos );
             $buffer = substr( $buffer, $pos + 1 );
             $line   = trim( $line ); // Remove any leading/trailing whitespace
             Util::logTrace( "Processing line: $line" );
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding a timeout mechanism is a significant enhancement that prevents potential infinite loops, improving the robustness and reliability of the code.

    9
    Improve error logging for unsupported file extensions

    Consider adding error logging for the case when the file extension is not supported.
    This will help with debugging and provide more context for the error.

    core/classes/actions/class.action.quickPick.php [465-468]

     } else {
    -    Util::logError('Unsupported file extension: ' . $fileExtension);
    -    return ['error' => 'Unsupported file extension'];
    +    $errorMessage = "Unsupported file extension: $fileExtension for file: $tmpFilePath";
    +    Util::logError($errorMessage);
    +    return ['error' => $errorMessage];
     }
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Enhancing error logging provides better debugging information, which is important for diagnosing issues related to unsupported file extensions.

    8
    Best practice
    Use a more descriptive variable name for clarity

    Consider using a more descriptive variable name instead of $result. For example,
    $fileRetrievalResult would better convey the purpose of this variable.

    core/classes/actions/class.action.quickPick.php [434]

    -$result = $bearsamppCore->getFileFromUrl($moduleUrl, $tmpFilePath, true);
    +$fileRetrievalResult = $bearsamppCore->getFileFromUrl($moduleUrl, $tmpFilePath, true);
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: The suggestion to use a more descriptive variable name improves code readability and maintainability, but it is not crucial for functionality.

    6
    Use a constant for the file reading chunk size

    Consider using a constant for the chunk size (8192) in the file reading loop. This
    would make the code more maintainable and allow for easy adjustments if needed.

    core/classes/class.core.php [512]

    -$buffer .= fread( $process, 8192 ); // Read in chunks of 8KB
    +const CHUNK_SIZE = 8192; // 8KB
    +$buffer .= fread( $process, self::CHUNK_SIZE );
     
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: Using a constant for the chunk size improves maintainability by making it easier to adjust the size if needed, but it is a minor improvement.

    5

    @jwaisner jwaisner merged commit 995a333 into main Aug 29, 2024
    1 check passed
    @jwaisner jwaisner deleted the ram branch August 29, 2024 00:55
    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