Scratch doesn't save tab order [$50]

[Mario Daniel Ruiz Saavedra]

1. Open various files with Scratch
2. Rearrange the tabs
3. Close & Open Scratch
4. Tabs rearrangement is not preserved

I think the exact state of the app should be preserved.

[SuperScript (superscript18)]

I have looked around at the behavior of this bug. It appears that the documents are arranged in the order of opening because of how they are stored in `DocumentView.docs`. I think that `DocumentView` needs to keep it's `.docs` GLib.List in order of tabs.

Also, the scroll position of each document is not preserved. I think `Document` needs a `get_state` method to retrieve it's path, scroll X/Y and current language. I am adding a bug for scroll position.

[quassy (quassy)]

The current cursor position / currently selected text should be remembered as well.

[SuperScript (superscript18)]

The linked branch didn't fix the problem.

[MarkyC (mmmarcooo)]

I tried implementing SuperScripts suggestion to have DocumentList keep the GLib.List that holds the Documents in order of the tabs.

This bug still persists, so there may be a bit more to do. Here is the code I've added thus far to on_doc_reordered as a starting point:

private void on_doc_reordered (Tab tab, int new_pos) {
    var doc = tab as Document;

    // Update the location in our list
    docs.remove (doc);
    docs.insert (doc, new_pos);

    doc.focus ();
}

as a gist:
https://gist.github.com/MarkyC/a2d57bacac32286ca591

[MarkyC (mmmarcooo)]

sorry, typo!

DocumentList should be DocumentView (doesn't look like you can edit comments on LP).

DocumentView has a GLib.List<Document>, which (I assume) holds the opened documents in scratch. My code keeps the ordering of this list in sync with the Notebook's ordering of the tabs.

[Robert Roth (evfool)]

@Markyc: it would be great if you could create a bzr branch and a merge proposal with your suggestion, that makes it easier to review and test, comment and approve.

[MarkyC (mmmarcooo)]

@Robert: Unfortunately my code doesn't actually fix the bug, it is simply a starting point (it updates the List in DocumentView.vala, which saves tab order for saved files, but not for temporary files).

I'm going to try and take another stab at this tonight and see if I can get it working for temp files as well

[Janne Lindström (janus-l)]

As far as i can tell nobody has found a fix for the temp documents yet so i took the liberty of having a look at the source. Im pretty new to elementary an vala but as far as i can tell the problem is in MainWindow.vala. The MainWindow constructor calls the method init_layout which in turn has an if/else clause at line 259 that loads temp docs before any of the other docs are loaded, hence placing them before all other documents in the tab order. I just commented out the if/else clause completely as i couldn't see a reason for it.

[MarkyC (mmmarcooo)]

@janus-l

See here: https://code.launchpad.net/~mmmarcooo/scratch/bugfix-1080904/+merge/257018

Initially, I removed the call to the action_open_temporary_files function, as that is what opens the temp documents first (as you've pointed out).

Then @cameronnemo pointed out that removing this line left him with a blank screen: https://code.launchpad.net/~mmmarcooo/scratch/bugfix-1080904/+merge/257018/comments/640451

I could not replicate that in my environment (removing the line worked flawlessly for me), but I trust Cameron (seeing as I don't know Vala or Elementary all that well), so I removed that fix. My PR is now just hanging on the vine.

[Janne Lindström (janus-l)]

@mmmarcooo

I was a bit too quick to post i think. I noticed some issues with my solution right after my previous post as well, same issues that @cameronnemo was describing plus some issues with the toolbar. I solved it by removing the if/else clause and replacing it with just the call to show_welcome. Here's a pastebin of the solution in case my explanation isn't very clear (line 259 is the relevant one).

http://pastebin.com/6LLTzda7

[Jeremy Wootten (jeremywootten)]

Shouldn't the two unapproved linked branches be unlinked and "retired" now a fix has been committed for this bug?