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

Feature Request - repeatSourceRange Functionality in Worksheet #3895

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

sibalonat
Copy link

@sibalonat sibalonat commented Feb 6, 2024

This is:

  • a bugfix
  • a new feature
  • refactoring
  • additional unit tests

Related to issue: #3894

Checklist:

  • Changes are covered by unit tests
  • Changes are covered by existing unit tests
  • New unit tests have been added
  • Code style is respected
  • Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
  • CHANGELOG.md contains a short summary of the change and a link to the pull request if applicable
  • Documentation is updated as necessary

Why this change is needed?

Provide an explanation of why this change is needed, with links to any Issues (if appropriate).
If this is a bugfix or a new feature, and there are no existing Issues, then please also create an issue that will make it easier to track progress with this PR.

Description:
This pull request introduces a new feature in the Worksheet class, the repeatSourceRange function. This function allows users to repeat a specified source range of cells multiple times in the worksheet. This is particularly useful when there is a need to duplicate a certain pattern or format of cells in a worksheet.

The repeatSourceRange function takes three parameters:

  1. $worksheet: The worksheet where the source range is located and where the repetitions will be placed.

  2. $sourceRange: The range of cells to be copied.

  3. $repetitions: The number of times the source range should be repeated.

  4. $groupSize: The number of rows in each group of repetitions.

Benefits:

  • Efficiency: This feature saves users from the tedious task of manually copying and pasting a range of cells multiple times. It automates the process, making it more efficient and less error-prone.

  • Consistency: By using this function, users can ensure that the repeated cells are identical, maintaining consistency in the worksheet.

  • Flexibility: This function is flexible and can be used in a variety of scenarios, such as creating multiple instances of a template within a worksheet, duplicating patterns, and more.

  • Ease of Use: This function is easy to use and integrates seamlessly with the existing PhpSpreadsheet library. Users can easily incorporate it into their projects without any major changes to their existing code.

Changes:
The main change is the addition of the repeatSourceRange function in the Worksheet class. Unit tests have also been added to ensure the correct functionality of this feature.

Testing:
Unit tests have been added to test the repeatSourceRange function. The tests cover basic functionality, such as copying cells and merged cells, and ensure that the cells are copied to the correct locations.

Please review and let me know if there are any changes or improvements needed

@sibalonat sibalonat closed this Feb 6, 2024
@sibalonat sibalonat reopened this Feb 6, 2024
@PowerKiKi
Copy link
Member

I don't understand how this works, or what is it supposed to do. So I checked your unit tests, but if I comment out $worksheet->repeatSourceRange($sourceRange, $repetitions, $groupSize);, then the test still pass, proving that your test does not test your code at all. So your tests need to be fixed.

How is this different than Worksheet::copyCells() ?

@sibalonat
Copy link
Author

sibalonat commented May 12, 2024

Thank you very much, @PowerKiKi Adrien, thank you for asking. I added this feature because of the limitation copyCells have by default. for sure in Excel, you can merge cells, but you can also copy merged cells to another place, given that the cells will maintain the same copy attribute. The logic, behind this, is that it could allow copying range cells programmatically, merged or non-merged, into a new location, but I also added higher functionality, at least as I faced a project from where this feature was inspired, to create copies by grinding (spread into certain position separated by space). For the tests, I'll take a look today most likely to see what can be improved there. Thanks again

@PowerKiKi
Copy link
Member

Copying the merged status of cells is a good improvement that could be accepted into PhpSpreadsheet. But repeating the copied cells sounds too specific to your use-case to live in PhpSpreadsheet. It won't get accepted.

If you want "Copy the merged status of cells" in PhpSpreadsheet, then it should probably live within Worksheet::copyCells(), doesn't it ?

So copyCells() could copy a single cell, like now, or copy a range of cells (and their merged status), depending what is given in $fromCell.

@sibalonat
Copy link
Author

@PowerKiKi it certainly makes sense what you wrote. The implementation I've written here, It's too specific, and thinking about it now, I have the same feeling. I could change it so that it only merges the cells that are (conventionally calling them simple) and the ranged cells where you have some range cells you want to copy. Thank you for the feedback

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

Successfully merging this pull request may close these issues.

None yet

2 participants