Skip to content

Bug: Fix Save As duplicate editor tabs; minor robustness improvements#479

Open
LinuxBoy-96 wants to merge 7 commits intopop-os:masterfrom
LinuxBoy-96:path
Open

Bug: Fix Save As duplicate editor tabs; minor robustness improvements#479
LinuxBoy-96 wants to merge 7 commits intopop-os:masterfrom
LinuxBoy-96:path

Conversation

@LinuxBoy-96
Copy link
Copy Markdown

@LinuxBoy-96 LinuxBoy-96 commented Dec 28, 2025 •
edited
Loading

This PR addresses a reproducible issue in Save As handling and includes a small set of related robustness improvements.

1) Bug fix: Save As could create duplicate editor tabs for the same file

Steps to reproduce:

  • Open test1.rs and test2.rs in separate editor tabs
  • In the test1.rs tab, use Save As… and select the existing path test2.rs
  • Both tabs could then point to the same on-disk file, allowing conflicting edits
  • The last save would silently overwrite the other (data loss)

Fix:

  • Detect when the Save As target path is already open in another editor tab
  • Switch to the existing tab instead of allowing duplicate tabs
  • This ensures a single source of truth per file and prevents silent data loss

2) Robustness improvements (no user-visible behavior change)

  • Make path handling more defensive to avoid fragile internal states
  • Improve consistency when updating tab titles and tab selection
  • Avoid relying on the active tab when an operation targets a specific tab entity

Notes:

  • User-facing behavior remains unchanged except for preventing the duplicate-tab Save As case
  • This PR currently includes additional commits related to robustness; if preferred, the Save As fix can be split into a minimal standalone PR

@jackpot51
Copy link
Copy Markdown
Member

jackpot51 commented Dec 30, 2025

This seems like more code change than would be needed, can you provide a way for me to replicate a bug that this PR fixes?

@LinuxBoy-96
Copy link
Copy Markdown
Author

LinuxBoy-96 commented Dec 30, 2025 •
edited
Loading

This seems like more code change than would be needed, can you provide a way for me to replicate a bug that this PR fixes?

Yeah sorry, now it make a little bit more sense... I did not found a speciffic bug, but this would be more robust now with my 2nd commit.

The code is overall cleaner and shorter.

I tested it and it work perfectly.

@LinuxBoy-96
Copy link
Copy Markdown
Author

LinuxBoy-96 commented Dec 30, 2025 •
edited
Loading

@jackpot51 fair point.
I don’t have a minimal user-visible repro, and I understand the concern about the size of the diff.
The goal here is improving internal robustness and consistency without changing behavior.
I’m happy to make targeted adjustments if there are specific parts you’d like trimmed or revised.

@LinuxBoy-96 LinuxBoy-96 changed the title Normalize file paths and stabilize tab updates improves internal path handling and stabilizes tab updates while keeping user behavior unchanged. Dec 30, 2025
@LinuxBoy-96
Copy link
Copy Markdown
Author

LinuxBoy-96 commented Jan 5, 2026

@jackpot51 If you have specific scenarios you’d like me to test (path edge cases, tab selection, Save/Save As, symlinks, etc.), I’m happy to run them and report back.

So far, all the cases I tested behaved correctly and matched current behavior.
If this refactor is not needed or not desirable right now, please feel free to close the PR. 😔

@LinuxBoy-96 LinuxBoy-96 changed the title improves internal path handling and stabilizes tab updates while keeping user behavior unchanged. Bug: Fix Save As duplicate editor tabs; minor robustness improvements Jan 5, 2026
@LinuxBoy-96
Copy link
Copy Markdown
Author

LinuxBoy-96 commented Jan 5, 2026 •
edited
Loading

@jackpot51 @mmstick Sorry for the extra ping.

I’ve now identified a concrete, reproducible issue that was causing real problems:
Save As could create multiple editor tabs pointing to the same file, leading to silent data loss.

This is fixed in the latest commit, so this PR now contains an actual bug fix.
I’d appreciate it if the workflow could be approved so CI can run.

While working on this, I knew something was off but couldn’t immediately pinpoint the root cause.
The earlier commits were part of narrowing things down and cleaning up the code until the issue I've experianced became clear.

I knew something was fishy but could exactly tell what I did wrong on my side while I was working. so the first few commit are only me narrowing and cleaning the code until I understood.

Thanks a lot.

@LinuxBoy-96 LinuxBoy-96 mentioned this pull request Jan 8, 2026
Open
Lcstyle pushed a commit to Lcstyle/cosmic-edit that referenced this pull request Jan 23, 2026
Merge PR pop-os#479 (Save As fix) - integrating key features manually
0758525
Lcstyle pushed a commit to Lcstyle/cosmic-edit that referenced this pull request Jan 23, 2026
@claude
Fix large file memory explosion (Issue pop-os#457)
31a36bf 
Root cause: cosmic-text's shape_until_scroll() uses INFINITY as scroll_end
when buffer height is not set, causing ALL lines to be shaped on file load.
Each character requires ~200 bytes of glyph/layout data, so a 60MB file
was consuming 18.8GB RAM (313x multiplier).

Fix: Set a minimal buffer height (100px) before loading files >1MB.
This limits initial shaping to ~5 visible lines instead of all lines.
The proper height is set during rendering, and additional lines are
shaped on-demand as user scrolls.

Results:
- Before: 60MB file -> 18.8GB RAM (crashes on 8-16GB systems)
- After:  60MB file -> 0.2GB RAM (94x memory reduction)

Also includes PR pop-os#479 Save As fix:
- Added resolve_path() helper for canonical path resolution
- Added set_path() and save_as() methods to EditorTab
- SaveAsResult now checks if file is already open in another tab

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@Lcstyle Lcstyle mentioned this pull request Jan 23, 2026
Closed
6 tasks
Lcstyle added a commit to Lcstyle/cosmic-edit that referenced this pull request Jan 23, 2026
@Lcstyle @claude
Update build config for forked cosmic-text and add build instructions
db9eca6 
- Update Cargo.toml to use Lcstyle/cosmic-text fork with rope-buffer feature
- Add patch section to redirect all cosmic-text dependencies to fork
- Add comprehensive FORK_BUILD_INSTRUCTIONS.md documenting:
  - All patches required (cosmic-text, onig)
  - Features included (PR pop-os#497, pop-os#479, large file fixes)
  - Troubleshooting common build issues
- Include fresh Cargo.lock with correct dependency resolution

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Lcstyle added a commit to Lcstyle/cosmic-edit that referenced this pull request Jan 23, 2026
@Lcstyle @claude
Merge rope buffer and scrollbar improvements
404f697 
Integrates feature/rope-buffer-integration branch which adds:
- RopeBuffer for efficient viewing of very large files (>1MB)
- Scrollbar improvements with proper thumb sizing for large files
- Line number offset support for windowed buffer display
- ScrollChanged message handler for incremental window refresh

Combined with existing features:
- Session restore + auto-save (PR pop-os#497)
- Save As duplicate tab fix (PR pop-os#479)
- Large file memory fix (Issue pop-os#457)
- Skip content hash for large files

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@LinuxBoy-96
Copy link
Copy Markdown
Author

LinuxBoy-96 commented Mar 2, 2026

@jackpot51 Any chance this can be merged :( I'm suffering here.

@jacobgkau jacobgkau requested review from a team March 2, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@LinuxBoy-96 @jackpot51