Properly handle both regularly saved files and files automatically saved on hot exit

[64kramsystem]

Due to hot exit events not currently made availble by ST, it's necessary to capture close events.

This causes a problem on dirty views, as it's not possible to distinguish a user-initiated close from a hot exit:

• in the former case, we don't want to save the fold data
• in the latter case, we do

There is no workaround possible using ST events; this solution stores the folding data on a per-view content basis (using a checksum), so that on open, the plugin will pick the fold data corresponding to the view content loaded.

On close events, previous fold data versions are kept. On save, only the latest one is kept, since it's guaranteed that on open, the saved version of the file will be opened.

The patch handles previous folds stored with the previous plugin version.

Closes https://github.com/RIDE-2DAY/AutoFoldCode/issues/10 and https://github.com/RIDE-2DAY/AutoFoldCode/issues/9.

[64kramsystem]

@acheronfail this is a working preview of the solution; let me know if the solution works for you, in which case we can iterate the code.

[acheronfail]

I like your solution so far 😄 thanks for the Pull Request!

[64kramsystem]

Also, rather than patching the save file format on the fly, let's make the file format versioned. Could you change the format to something like:

{
  // Let's start the version at 1
  "version": 1,
  // This is your proposed new storage file format
  "folds": {
    "<filename>":
    {
      "<content_crc32>":
      [
        [
          <region_start>,
          <region_end>
        ],
        ...
      ],
      ...
    },
    ...
  }
}

And then on plugin init can you define a function that checks if settings["version"] is defined and performs a data migration?

This way we don't have to reason about file format changes in the plugin's implementation itself.

What should the plugin do, in this case, when reading a setting file that it's the old version?

Since in this case the setting is monolithic, there are two choices:

1. erase the content
2. convert each entry to the new format

while strategy 2. is appealing, there is the problem that the format requires knowledge of the checksum of the file. This implies that on reading a setting file of the old format, each file stored should be read in order to compute the checksum. the downside is that the lag in this case grows linearly with the number of files stored (and it's async, so it may be applied with a very noticeable delay), the upside is that it happens only once.

if we're going for 2., but reading all the files is undesirable, a potential strategy is to use a "wildcard" checksum. this would be always applied and save reading all the files during conversion. of course, this is an optimization, and the usual considerations apply.

[64kramsystem]

I like your solution so far thanks for the Pull Request!

Great! Will work on in in the few next days.

[acheronfail]

What should the plugin do, in this case, when reading a setting file that it's the old version?

Let's not make this overly complicated, this is just a small plugin for a text editor which is maintaining code folds, so at worst it could be a minor annoyance if anything goes wrong. 😆

When we detect a storage file that's not the correct version, I'd recommend using yes_no_cancel_dialog (see: https://www.sublimetext.com/docs/3/api_reference.html#sublime, you can set the button text using yes_title and no_title) and giving the user two choices:

1. Reset all folds (clear everything and start with a new file)
2. Migrate fold data (inform the user that the plugin will attempt to read each of the files to generate a checksum for them)

Some users might not want the plugin to go an access previous files, so we should default to option 1 (resetting all folds).

[64kramsystem]

@acheronfail can you give a review of the current prototype, please? if it's ok, I'll follow up.

Something odd that I've noticed is that sublime.Settings doesn't support retrieving all the settings. If the API is as documented, it's impossible to read all the settings in the old version and process them, since the setting keys (filenames) are not known o_O

[acheronfail]

Something odd that I've noticed is that sublime.Settings doesn't support retrieving all the settings. If the API is as documented, it's impossible to read all the settings in the old version and process them, since the setting keys (filenames) are not known o_O

If it doesn't support retrieving the settings then I guess there's nothing we can do.

Since nobody will live or die depending on this plugin, I would just recommend we keep our plugin code as simple as possible - it makes it much easier to maintain.

In that spirit, I definitely see a benefit to having the new storage format, so just go ahead and delete the storage file so we can re-create it anew.

[64kramsystem]

OK, ready for review :+1:

[acheronfail]

This looks good, thanks for all the iteration and work on it @saveriomiroddi!