Open document with range: Ensure view scrolls to selection

[jeremypw]

Fixes #1703

It seems that scrolling the view must be done after Gtk has finished calculating the line heights of the displayed documents. This is achieved by doing the scrolling in a low priority thread.

Opportunity was taken to reduce code repetition.

This fix is extracted from #1702 .

[Dipanshusinghh]

Hi @jeremypw, thanks for the fix! The logic to delay scrolling using Idle.add_full makes perfect sense.
While reviewing the code, I noticed a couple of minor things:-

1. Duplicate code: In the new after_insert_document() function, doc.focus(); is called twice in a row. One of them can be safely removed/
2. Namespace consistency: The doc parameter in after_insert_document uses the legacy Scratch.Services.Document type, while open_document just uses Services.Document. It might be good to remove the Scratch. prefix to keep it consistent./

Great work on reducing the code repetition! Let me know if you'd like me to test this branch locally.

[Dipanshusinghh]

Hi @jeremypw, I checked the latest merge update and the refactor around after_insert_document() looks cleaner now.
One thing I noticed in the conflict resolution: save_opened_files (); seems to have been removed from this flow in DocumentView.vala. Was that intentional? I’m just wondering whether the opened document/session state is now persisted somewhere else after these changes, since previously this path appeared to handle it directly.
The delayed scrolling approach with Idle.add_full still looks like the right fix for the GTK timing issue though.

[jeremypw]

@Dipanshusinghh I have not addressed you specific comments yet - just merged master.

[jeremypw]

I intended to include update_opened_files_settings - looks like that got missed. Thanks for noticing that.

[Dipanshusinghh]

Glad I could help catch that 🙂 The rest of the refactor looks good so far --- I willl re-check once the missing update is added./

[Dipanshusinghh]

Thanks for addressing the review comments @jeremypw — the follow-up commits look good to me now. The namespace cleanup and the re-added update_opened_files_settings call make the refactor much cleaner overall,.

[Dipanshusinghh]

Thanks for addressing the follow-up feedback @jeremypw — the latest cleanup looks good to me. The namespace simplification keeps the method signature cleaner, and the scrolling/selection handling now feels much more consistent overall.

[Dipanshusinghh]

I noticed that save_opened_files (); was removed during the conflict resolution here. Was that intentional? It looks like this path previously handled persisting the opened document state, so I just wanted to double-check that this behavior is still covered elsewhere after the refactor.