Skip to content

Comments

Rework code/formula translation & name-updating#2354

Merged
ddimaria merged 33 commits intoqafrom
ajf/refactor-update-code-cell-references
Mar 24, 2025
Merged

Rework code/formula translation & name-updating#2354
ddimaria merged 33 commits intoqafrom
ajf/refactor-update-code-cell-references

Conversation

@HactarCE
Copy link
Contributor

@HactarCE HactarCE commented Feb 28, 2025

This PR overhauls sheet/table/column renaming and coordinate translation in formulas and code cells.

Intended behavior

These actions should preserve all cell references:

  • Cutting & pasting code cells

These actions should update only relative references in code & formulas to all sheets:

  • Copying & pasting code cells

These actions should update all references in code & formulas to the sheet where they occur:

  • Inserting/deleting columns
  • Renaming a sheet
  • Renaming a table
  • Renaming a column within a table

References should be updated even in cells that are not in the current sheet. References (including invalid ones) should remain unmodified.

Let me know via comment here or on Slack if I've made any mistakes in the intended behavior.

Code review

For code review, check that there are no extraneous calls to Grid::a1_context().

@qa-wolf
Copy link

qa-wolf bot commented Feb 28, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

@cla-bot cla-bot bot added the cla-signed label Feb 28, 2025
@HactarCE HactarCE force-pushed the ajf/refactor-update-code-cell-references branch from 03f1a89 to 89df9fa Compare February 28, 2025 16:41
@HactarCE HactarCE force-pushed the ajf/refactor-update-code-cell-references branch from 89df9fa to 9942fd1 Compare February 28, 2025 16:42
@github-actions
Copy link
Contributor

github-actions bot commented Feb 28, 2025

Preview - Build & Deploy Images

✅ Build images
✅ Deploy images

🕒 Last deployed: Mar 24, 2025 at 05:20 PM UTC

🔗 URL: https://ajf-refactor-update-code-cell-references.quadratic-preview.com

@github-actions github-actions bot temporarily deployed to preview-pr-2354 February 28, 2025 16:46 Destroyed
@codecov
Copy link

codecov bot commented Feb 28, 2025

Codecov Report

Attention: Patch coverage is 96.73540% with 38 lines in your changes missing coverage. Please review.

Project coverage is 90.40%. Comparing base (a9e8d04) to head (c19990d).
Report is 38 commits behind head on qa.

Files with missing lines Patch % Lines
quadratic-core/src/formulas/parser/mod.rs 76.92% 21 Missing ⚠️
quadratic-core/src/a1/sheet_cell_ref_range.rs 84.78% 7 Missing ⚠️
quadratic-core/src/grid/code_cell.rs 97.99% 5 Missing ⚠️
quadratic-core/src/a1/a1_selection/mutate.rs 96.82% 2 Missing ⚠️
quadratic-core/src/a1/cell_ref_range/mutate.rs 97.43% 1 Missing ⚠️
quadratic-core/src/a1/js_selection/create.rs 0.00% 1 Missing ⚠️
quadratic-core/src/grid/sheets.rs 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               qa    #2354      +/-   ##
==========================================
+ Coverage   88.47%   90.40%   +1.92%     
==========================================
  Files         382      382              
  Lines       85129    85354     +225     
==========================================
+ Hits        75316    77161    +1845     
+ Misses       9813     8193    -1620     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions bot temporarily deployed to preview-pr-2354 February 28, 2025 16:50 Destroyed
@github-actions github-actions bot temporarily deployed to preview-pr-2354 February 28, 2025 19:01 Destroyed
@github-actions github-actions bot temporarily deployed to preview-pr-2354 February 28, 2025 20:36 Destroyed
@github-actions github-actions bot temporarily deployed to preview-pr-2354 February 28, 2025 20:54 Destroyed
@HactarCE HactarCE marked this pull request as draft March 10, 2025 20:40
@github-actions github-actions bot temporarily deployed to preview-pr-2354 March 10, 2025 20:52 Destroyed
@github-actions github-actions bot temporarily deployed to preview-pr-2354 March 10, 2025 21:09 Destroyed
@github-actions github-actions bot temporarily deployed to preview-pr-2354 March 10, 2025 21:11 Destroyed
@github-actions github-actions bot temporarily deployed to preview-pr-2354 March 20, 2025 17:04 Destroyed
@HactarCE HactarCE marked this pull request as ready for review March 20, 2025 21:04
@github-actions github-actions bot temporarily deployed to preview-pr-2354 March 20, 2025 21:05 Destroyed

/// Constructs an A1 context with only a single sheet.
pub fn with_single_sheet(sheet_name: &str, sheet_id: SheetId) -> Self {
let mut ret = Self::default();
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does ret stand for here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"return value"

I avoid "res" or "result" because Result has special meaning in rust

I'm happy to change this variable if you have another name in mind

Copy link
Collaborator

@ddimaria ddimaria left a comment

Choose a reason for hiding this comment

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

@AyushAgrawal-A2 just one comment to address before merging

@github-actions github-actions bot temporarily deployed to preview-pr-2354 March 21, 2025 22:02 Destroyed
@github-actions github-actions bot temporarily deployed to preview-pr-2354 March 21, 2025 22:02 Destroyed
Copy link
Collaborator

@davidkircos davidkircos left a comment

Choose a reason for hiding this comment

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

nice work on this!

@ddimaria ddimaria merged commit b18667d into qa Mar 24, 2025
24 checks passed
@ddimaria ddimaria deleted the ajf/refactor-update-code-cell-references branch March 24, 2025 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants