Skip to content

Add per-sheet filtering to ExcelToPdfConverter (SheetIndices / SheetNames)#44

Open
Copilot wants to merge 2 commits intomainfrom
copilot/finish-task-11
Open

Add per-sheet filtering to ExcelToPdfConverter (SheetIndices / SheetNames)#44
Copilot wants to merge 2 commits intomainfrom
copilot/finish-task-11

Conversation

Copy link

Copilot AI commented Feb 24, 2026

ExcelToPdfConverter had no way to select a subset of sheets — it always converted every sheet. This adds two mutually-exclusive filter options to ConversionOptions.

API changes

ExcelToPdfConverter.ConversionOptions — two new optional properties:

Property Type Behaviour
SheetIndices IReadOnlyList<int>? Include only the specified zero-based sheet indices
SheetNames IReadOnlyList<string>? Include only sheets matching by name (case-insensitive)

SheetIndices takes precedence when both are set. null / empty → all sheets (existing default behaviour unchanged).

// By index
var options = new ExcelToPdfConverter.ConversionOptions
{
    SheetIndices = new[] { 0, 2 },   // first and third sheet only
};

// By name (case-insensitive)
var options2 = new ExcelToPdfConverter.ConversionOptions
{
    SheetNames = new[] { "Sales", "Summary" },
};

var doc = ExcelToPdfConverter.Convert("data.xlsx", options);

Changes

  • ExcelToPdfConverter.cs — added SheetIndices / SheetNames to ConversionOptions; Convert(Stream, ...) filters the sheet list before rendering using a HashSet lookup for O(1) membership tests.
  • ExcelToPdfConverterTests.cs — 4 new tests covering index filtering, name filtering, precedence, and no-match fallback; added CreateMultiSheetExcel in-memory xlsx helper.
  • README.md — documented the new options with examples.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: shps951023 <12729184+shps951023@users.noreply.github.com>
Copilot AI changed the title [WIP] Complete implementation of task #11 Add per-sheet filtering to ExcelToPdfConverter (SheetIndices / SheetNames) Feb 24, 2026
@shps951023 shps951023 marked this pull request as ready for review February 27, 2026 01:19
Copilot AI review requested due to automatic review settings February 27, 2026 01:19
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds optional per-sheet filtering to ExcelToPdfConverter so callers can convert only selected sheets instead of always rendering the full workbook.

Changes:

  • Added SheetIndices and SheetNames to ExcelToPdfConverter.ConversionOptions and filtered the sheet enumeration accordingly.
  • Added xUnit tests covering index filtering, name filtering (case-insensitive), precedence, and no-match behavior.
  • Documented the new options in the README with usage examples.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/MiniPdf/ExcelToPdfConverter.cs Adds SheetIndices / SheetNames options and filters sheets before rendering.
tests/MiniPdf.Tests/ExcelToPdfConverterTests.cs Adds multi-sheet XLSX test helper + new tests for sheet filtering and precedence.
README.md Documents multi-sheet selection options and examples.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

```

When both `SheetIndices` and `SheetNames` are specified, `SheetIndices` takes precedence.
```
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

There is an extra stray Markdown code fence after the precedence sentence, which starts a new (unclosed) code block and breaks the README formatting. Remove the trailing ``` on this line (or ensure code fences are correctly paired).

Suggested change
```

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants