[PR #518] [CLOSED] Add workbook_rename_worksheet() and workbook_remove_worksheet() #525

New issue

Closed

opened 2026-10-02 23:12:47 -06:00 by gitea-mirror · 0 comments

gitea-mirror commented

2026-10-02 23:12:47 -06:00

Owner

📋 Pull Request Information

Original PR: https://github.com/jmcnamara/libxlsxwriter/pull/518
Author: @viest
Created: 6/14/2026
Status: ❌ Closed

Base: main ← Head: rename

📝 Commits (1)

499aba4 workbook: add worksheet rename and remove functions

📊 Changes

6 files changed (+482 additions, -0 deletions)

View changed files

📝 include/xlsxwriter/common.h (+3 -0)
📝 include/xlsxwriter/workbook.h (+48 -0)
📝 src/utility.c (+1 -0)
📝 src/workbook.c (+127 -0)
➕ test/unit/workbook/test_workbook_remove_worksheet.c (+156 -0)
➕ test/unit/workbook/test_workbook_rename_worksheet.c (+147 -0)

📄 Description

Summary

Adds two public workbook functions, both looking the worksheet up by name (case insensitive, matching workbook_get_worksheet_by_name()):

workbook_rename_worksheet(workbook, old_name, new_name)
workbook_remove_worksheet(workbook, name)

Also adds a new LXW_ERROR_SHEETNAME_NOT_FOUND error code for the lookup-miss case.

Motivation

Downstream language bindings sometimes need to rename or drop a worksheet after it has been added. This came up for the PHP extension php-ext-xlswriter#306, where rename/delete of sheets is a frequently requested feature. These functions let bindings implement that without reaching into private structs.

Design notes

Rename detaches the name-tree node before validating new_name, so a case-only rename such as Sheet1 -> sheet1 is not treated as a collision. It preserves the single shared worksheet->name / name-node allocation used by workbook_add_worksheet().
Remove detaches the worksheet from the worksheets list, the combined sheets list and the name tree; frees the worksheet; then renumbers the remaining sheet indexes so they stay contiguous and shifts the active_sheet/first_sheet positions past the removed sheet (resetting them to 0 if the removed sheet was active/first, mirroring worksheet_hide()).
The operations are purely additive — no existing function or behavior changes. Rename/remove should be carried out before a worksheet is referenced by formulas, defined names or charts in other worksheets (documented in the doxygen notes), the same constraint Excel itself has when deleting a sheet.

Verification

New unit tests: 9 rename cases, 8 remove cases (test/unit/workbook/).
Mutation-tested the bug-prone paths: case-only rename (detach-before-validate), not-found return, index renumbering, and active/first-sheet shift each have a test that fails when the corresponding logic is removed.
Full unit suite passes (455 tests across all modules, no regressions from the new error-code enum value).
Compiles clean under -Wall -Wextra -Wstrict-prototypes -pedantic -ansi.
End-to-end smoke: created a workbook, renamed a middle sheet, removed another middle sheet, set the active sheet to the last one, and closed — the generated workbook.xml has contiguous sheetId/r:id, the correct three sheetN.xml parts, and activeTab correctly tracking the shifted sheet.

I'm happy to split this into a rename-only PR and a remove-only PR, add a Changes.txt entry, or add the chartsheet counterparts (workbook_rename/remove_chartsheet, structurally identical) if that's preferred.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/jmcnamara/libxlsxwriter/pull/518 **Author:** [@viest](https://github.com/viest) **Created:** 6/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `rename` --- ### 📝 Commits (1) - [`499aba4`](https://github.com/jmcnamara/libxlsxwriter/commit/499aba47c55dc6286582c9a686f5814572505208) workbook: add worksheet rename and remove functions ### 📊 Changes **6 files changed** (+482 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `include/xlsxwriter/common.h` (+3 -0) 📝 `include/xlsxwriter/workbook.h` (+48 -0) 📝 `src/utility.c` (+1 -0) 📝 `src/workbook.c` (+127 -0) ➕ `test/unit/workbook/test_workbook_remove_worksheet.c` (+156 -0) ➕ `test/unit/workbook/test_workbook_rename_worksheet.c` (+147 -0) </details> ### 📄 Description ## Summary Adds two public workbook functions, both looking the worksheet up by name (case insensitive, matching `workbook_get_worksheet_by_name()`): - `workbook_rename_worksheet(workbook, old_name, new_name)` - `workbook_remove_worksheet(workbook, name)` Also adds a new `LXW_ERROR_SHEETNAME_NOT_FOUND` error code for the lookup-miss case. ## Motivation Downstream language bindings sometimes need to rename or drop a worksheet after it has been added. This came up for the PHP extension [php-ext-xlswriter#306](https://github.com/viest/php-ext-xlswriter/issues/306), where rename/delete of sheets is a frequently requested feature. These functions let bindings implement that without reaching into private structs. ## Design notes - **Rename** detaches the name-tree node *before* validating `new_name`, so a case-only rename such as `Sheet1` -> `sheet1` is not treated as a collision. It preserves the single shared `worksheet->name` / name-node allocation used by `workbook_add_worksheet()`. - **Remove** detaches the worksheet from the worksheets list, the combined sheets list and the name tree; frees the worksheet; then **renumbers the remaining sheet indexes** so they stay contiguous and **shifts the `active_sheet`/`first_sheet` positions** past the removed sheet (resetting them to 0 if the removed sheet was active/first, mirroring `worksheet_hide()`). - The operations are purely additive — no existing function or behavior changes. Rename/remove should be carried out before a worksheet is referenced by formulas, defined names or charts in other worksheets (documented in the doxygen notes), the same constraint Excel itself has when deleting a sheet. ## Verification - New unit tests: 9 rename cases, 8 remove cases (`test/unit/workbook/`). - Mutation-tested the bug-prone paths: case-only rename (detach-before-validate), not-found return, index renumbering, and active/first-sheet shift each have a test that fails when the corresponding logic is removed. - Full unit suite passes (455 tests across all modules, no regressions from the new error-code enum value). - Compiles clean under `-Wall -Wextra -Wstrict-prototypes -pedantic -ansi`. - End-to-end smoke: created a workbook, renamed a middle sheet, removed another middle sheet, set the active sheet to the last one, and closed — the generated `workbook.xml` has contiguous `sheetId`/`r:id`, the correct three `sheetN.xml` parts, and `activeTab` correctly tracking the shifted sheet. I'm happy to split this into a rename-only PR and a remove-only PR, add a Changes.txt entry, or add the chartsheet counterparts (`workbook_rename/remove_chartsheet`, structurally identical) if that's preferred. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

gitea-mirror

2026-10-02 23:12:47 -06:00