Skip to content

Update skill with new features#24

Open
nfcampos wants to merge 1 commit intomainfrom
nc/3mar/next
Open

Update skill with new features#24
nfcampos wants to merge 1 commit intomainfrom
nc/3mar/next

Conversation

@nfcampos
Copy link
Contributor

@nfcampos nfcampos commented Mar 3, 2026

No description provided.

Copilot AI review requested due to automatic review settings March 3, 2026 16:15
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

Updates the xlsx-code-mode skill documentation to cover newly supported Excel workbook operations (scenario sweeps, conditional formatting, range utilities) and reflects the replacement of table detection with a richer sheet description API.

Changes:

  • Added Quick Reference examples for scenarios and conditional formatting.
  • Updated API reference tables and TypeScript-shaped docs to include new operations (describeSheets, findAndReplace, autoFitColumns, sortRange, copyRange, conditional formatting APIs) and enriched read/set cell metadata (note, link, thread).
  • Updated CHANGELOG.md to document new features and the detectTablesdescribeSheets breaking change.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
skills/xlsx-code-mode/SKILL.md Adds examples and expands the documented xlsx API surface (scenarios, CF, range ops, metadata fields).
CHANGELOG.md Captures the unreleased feature set and the breaking API rename.

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

interface CfWritableRule extends CfRuleShared {
type: CfWritableRuleType;
}
type CfRule = CfReadableRule;
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

type CfRule = CfReadableRule; is misleadingly named (it reads like a union of readable+writable rules, but it aliases only the readable variant) and isn’t used by any signatures here. Consider removing it, or redefining it as an explicit union/type that matches intended usage to avoid confusion in the generated typings/docs.

Suggested change
type CfRule = CfReadableRule;

Copilot uses AI. Check for mistakes.
Comment on lines +672 to +691
/** Describe all visible sheets with detected tables and structure maps. */
function describeSheets(wb): Promise<
Record<
string,
{
/** Full range covering the row headers + column headers + data rows */
address: string;
/** Top labels as TSV with addresses (format: ColRow|Value\tColRow|Value) */
headerRows: string;
/** Side labels as TSV with addresses (format: ColRow|Value\nColRow|Value), null when no row labels */
headerCols: string | null;
/** Excel table name for Data Tables, absent for heuristic-detected tables */
tableName?: string;
tables: Record<
string,
{
/** Full range covering the row headers + column headers + data rows */
address: string;
/** Top labels as TSV with addresses (format: ColRow|Value\tColRow|Value) */
headerRows: string;
/** Side labels as TSV with addresses (format: ColRow|Value\nColRow|Value), null when no row labels */
headerCols: string | null;
/** Excel table name for Data Tables, absent for heuristic-detected tables */
tableName?: string;
}
>;
/** Compact ASCII structure map showing cell types per row */
structure: string;
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

describeSheets returns tables: Record<string, …> but the meaning of the table record keys isn’t documented (e.g., stable table id vs. display label vs. Excel table name). This makes consuming the response ambiguous; consider either documenting what the keys represent and whether they’re stable across runs, or returning an array with explicit id/name fields.

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