Skip to content

fix(google-sheets): support multi-letter column names (AA, AH, ZZ, etc.) in range operations#1

Open
Copilot wants to merge 5 commits intomasterfrom
copilot/fix-multi-letter-columns
Open

fix(google-sheets): support multi-letter column names (AA, AH, ZZ, etc.) in range operations#1
Copilot wants to merge 5 commits intomasterfrom
copilot/fix-multi-letter-columns

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 20, 2026

WHY

Column-to-index conversion used charCodeAt(0) - 65, which silently truncates multi-letter columns — "AH" becomes "A" (index 0), "ZZ" becomes "Z" (index 25). Any range operation on columns past Z silently applies to the wrong column.

Root cause: 7 call sites across formatting, validation, merge, and protection actions all used the single-char hack instead of a proper base-26 decode.

Fix: Replaced all broken charCodeAt(0) usages with the existing _getColumnIndex method, which already implements the correct base-26 algorithm (A→1, Z→26, AA→27, AH→34, ZZ→702):

// Before (broken for multi-letter columns)
startColumnIndex: startCol.charCodeAt(0) - 65,
endColumnIndex:   endCol.charCodeAt(0) - 64,

// After
startColumnIndex: this.googleSheets._getColumnIndex(startCol) - 1, // 0-based
endColumnIndex:   this.googleSheets._getColumnIndex(endCol),       // exclusive end

Files updated:

  • google_sheets.app.mjsresetRowFormat fixed
  • update-formatting.mjs
  • merge-cells.mjs
  • add-conditional-format-rule.mjs
  • set-data-validation.mjs
  • update-conditional-format-rule.mjs
  • add-protected-range.mjs

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…ions

Co-authored-by: tomasmcm <6323928+tomasmcm@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix multi-letter column support in range parsing fix(google-sheets): support multi-letter column names (AA, AH, ZZ, etc.) in range operations Mar 20, 2026
Copilot AI requested a review from tomasmcm March 20, 2026 16:14
@tomasmcm tomasmcm requested a review from Copilot March 20, 2026 16:15
@tomasmcm tomasmcm marked this pull request as ready for review March 20, 2026 16:26
@tomasmcm tomasmcm requested review from Copilot and removed request for Copilot March 20, 2026 16:27
@tomasmcm
Copy link
Copy Markdown
Member

@copilot there is already a _getColumnIndex why don't we use that instead?

Copy link
Copy Markdown

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

This PR fixes Google Sheets range operations for columns beyond Z by replacing single-character ASCII math with a proper base-26 column-letter decoder, ensuring ranges like AA:AH map to the correct grid indices.

Changes:

  • Added _columnLetterToIndex(col) helper to convert multi-letter column names to 0-based indices.
  • Updated range-building logic to use _columnLetterToIndex across formatting, conditional formatting, data validation, merge, and protected range actions.
  • Adjusted end column handling to consistently use Google Sheets API’s exclusive endColumnIndex.

Reviewed changes

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

Show a summary per file
File Description
components/google_sheets/google_sheets.app.mjs Adds _columnLetterToIndex helper and updates resetRowFormat to support multi-letter columns.
components/google_sheets/actions/update-formatting/update-formatting.mjs Uses _columnLetterToIndex for correct start/end column indices in formatting updates.
components/google_sheets/actions/merge-cells/merge-cells.mjs Uses _columnLetterToIndex for correct column indices when merging ranges.
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs Uses _columnLetterToIndex so conditional formatting ranges work past column Z.
components/google_sheets/actions/set-data-validation/set-data-validation.mjs Uses _columnLetterToIndex so validation ranges work past column Z.
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs Uses _columnLetterToIndex for correct conditional format rule update ranges.
components/google_sheets/actions/add-protected-range/add-protected-range.mjs Uses _columnLetterToIndex so protected ranges work past column Z.

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

Comment on lines +124 to +130
*/
_columnLetterToIndex(col) {
if (!col) return 0;
let index = 0;
const upper = col.toUpperCase();
for (let i = 0; i < upper.length; i++) {
index = index * 26 + (upper.charCodeAt(i) - 64);
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

_columnLetterToIndex currently defaults to 0 when col is falsy, and it will also happily compute an index for non A–Z characters (e.g. $A$, whitespace), which can yield negative/incorrect indices and silently target the wrong range. Consider validating col (string, non-empty, ^[A-Z]+$ after normalization/stripping $) and throwing an error instead of defaulting to column A.

Suggested change
*/
_columnLetterToIndex(col) {
if (!col) return 0;
let index = 0;
const upper = col.toUpperCase();
for (let i = 0; i < upper.length; i++) {
index = index * 26 + (upper.charCodeAt(i) - 64);
* @throws {Error} If the column reference is not a non-empty A–Z string.
*/
_columnLetterToIndex(col) {
if (!isString(col)) {
throw new Error("Column reference must be a non-empty string");
}
// Normalize: remove absolute-reference markers, trim whitespace, and uppercase
const normalized = col.replace(/\$/g, "").trim().toUpperCase();
// Validate: must be non-empty and contain only A–Z characters
if (!normalized || !/^[A-Z]+$/.test(normalized)) {
throw new Error(`Invalid column reference: "${col}"`);
}
let index = 0;
for (let i = 0; i < normalized.length; i++) {
index = index * 26 + (normalized.charCodeAt(i) - 64);

Copilot uses AI. Check for mistakes.
Comment on lines +127 to +132
let index = 0;
const upper = col.toUpperCase();
for (let i = 0; i < upper.length; i++) {
index = index * 26 + (upper.charCodeAt(i) - 64);
}
return index - 1; // 0-based for Google Sheets API
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

This helper duplicates the existing _getColumnIndex(column) base-26 logic in this file (which already supports multi-letter columns). To reduce divergence risk, consider implementing _columnLetterToIndex in terms of _getColumnIndex (or consolidating to a single helper with clear 0-based vs 1-based semantics) and avoid the magic -64 constant for readability.

Suggested change
let index = 0;
const upper = col.toUpperCase();
for (let i = 0; i < upper.length; i++) {
index = index * 26 + (upper.charCodeAt(i) - 64);
}
return index - 1; // 0-based for Google Sheets API
// Delegate to the existing helper that converts a column letter to a 1-based index,
// then convert to a 0-based index for use with the Google Sheets API.
const oneBasedIndex = this._getColumnIndex(col);
return oneBasedIndex - 1;

Copilot uses AI. Check for mistakes.
…helper

Co-authored-by: tomasmcm <6323928+tomasmcm@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented Mar 20, 2026

@copilot there is already a _getColumnIndex why don't we use that instead?

Good call — replaced _columnLetterToIndex with the existing _getColumnIndex across all call sites and removed the new helper entirely. _getColumnIndex already handles multi-letter columns correctly; callers now use _getColumnIndex(col) - 1 for the 0-based start index and _getColumnIndex(col) for the exclusive end index. Committed in 267cf5e.

@tomasmcm
Copy link
Copy Markdown
Member

@copilot this got approved, can you run this command to bump the version codes?

sed -i 0,/0.1.14/{s/0.1.14/0.1.15/} components/google_sheets/actions/add-column/add-column.mjs && sed -i 0,/0.2.18/{s/0.2.18/0.2.19/} components/google_sheets/actions/add-multiple-rows/add-multiple-rows.mjs && sed -i 0,/2.2.1/{s/2.2.1/2.2.2/} components/google_sheets/actions/add-single-row/add-single-row.mjs && sed -i 0,/0.1.18/{s/0.1.18/0.1.19/} components/google_sheets/actions/clear-cell/clear-cell.mjs && sed -i 0,/0.1.16/{s/0.1.16/0.1.17/} components/google_sheets/actions/clear-rows/clear-rows.mjs && sed -i 0,/0.1.14/{s/0.1.14/0.1.15/} components/google_sheets/actions/copy-worksheet/copy-worksheet.mjs && sed -i 0,/0.1.16/{s/0.1.16/0.1.17/} components/google_sheets/actions/create-spreadsheet/create-spreadsheet.mjs && sed -i 0,/0.1.14/{s/0.1.14/0.1.15/} components/google_sheets/actions/create-worksheet/create-worksheet.mjs && sed -i 0,/0.0.1/{s/0.0.1/0.0.2/} components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs && sed -i 0,/0.0.14/{s/0.0.14/0.0.15/} components/google_sheets/actions/delete-rows/delete-rows.mjs && sed -i 0,/0.1.14/{s/0.1.14/0.1.15/} components/google_sheets/actions/delete-worksheet/delete-worksheet.mjs && sed -i 0,/0.2.17/{s/0.2.17/0.2.18/} components/google_sheets/actions/find-row/find-row.mjs && sed -i 0,/0.1.16/{s/0.1.16/0.1.17/} components/google_sheets/actions/get-cell/get-cell.mjs && sed -i 0,/0.0.2/{s/0.0.2/0.0.3/} components/google_sheets/actions/get-current-user/get-current-user.mjs && sed -i 0,/0.1.15/{s/0.1.15/0.1.16/} components/google_sheets/actions/get-spreadsheet-by-id/get-spreadsheet-by-id.mjs && sed -i 0,/0.1.16/{s/0.1.16/0.1.17/} components/google_sheets/actions/get-values-in-range/get-values-in-range.mjs && sed -i 0,/0.1.14/{s/0.1.14/0.1.15/} components/google_sheets/actions/insert-anchored-note/insert-anchored-note.mjs && sed -i 0,/0.1.15/{s/0.1.15/0.1.16/} components/google_sheets/actions/insert-comment/insert-comment.mjs && sed -i 0,/0.0.1/{s/0.0.1/0.0.2/} components/google_sheets/actions/insert-dimension/insert-dimension.mjs && sed -i 0,/0.1.14/{s/0.1.14/0.1.15/} components/google_sheets/actions/list-worksheets/list-worksheets.mjs && sed -i 0,/0.0.1/{s/0.0.1/0.0.2/} components/google_sheets/actions/move-dimension/move-dimension.mjs && sed -i 0,/0.1.16/{s/0.1.16/0.1.17/} components/google_sheets/actions/update-cell/update-cell.mjs && sed -i 0,/0.1.16/{s/0.1.16/0.1.17/} components/google_sheets/actions/update-multiple-rows/update-multiple-rows.mjs && sed -i 0,/0.1.18/{s/0.1.18/0.1.19/} components/google_sheets/actions/update-row/update-row.mjs && sed -i 0,/0.1.18/{s/0.1.18/0.1.19/} components/google_sheets/actions/upsert-row/upsert-row.mjs && sed -i 0,/0.1.4/{s/0.1.4/0.1.5/} components/google_sheets/sources/new-comment/new-comment.mjs && sed -i 0,/0.0.2/{s/0.0.2/0.0.3/} components/google_sheets/sources/new-comment-polling/new-comment-polling.mjs && sed -i 0,/0.2.4/{s/0.2.4/0.2.5/} components/google_sheets/sources/new-row-added/new-row-added.mjs && sed -i 0,/0.1.4/{s/0.1.4/0.1.5/} components/google_sheets/sources/new-row-added-polling/new-row-added-polling.mjs && sed -i 0,/0.3.5/{s/0.3.5/0.3.6/} components/google_sheets/sources/new-updates/new-updates.mjs && sed -i 0,/0.0.3/{s/0.0.3/0.0.4/} components/google_sheets/sources/new-updates-polling/new-updates-polling.mjs && sed -i 0,/0.2.4/{s/0.2.4/0.2.5/} components/google_sheets/sources/new-worksheet/new-worksheet.mjs && sed -i 0,/0.0.2/{s/0.0.2/0.0.3/} components/google_sheets/sources/new-worksheet-polling/new-worksheet-polling.mjs

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