Jump-to-definition doesn't work on not-zipped source archives.

[jcreedcmu]

Databases from LGTM are built with odasa rather than codeql, and so don't have zipped source archives, and require an extra codeql database cleanup step to zip the source archive before jump-to-definition works on them.

It would be nice in principle if it just worked out of the box. With the current strategy of making one DefinitionProvider for the zipped archive scheme, it doesn't seem possible. We could perhaps, for each database add and delete, register and unregister a DefinitionProvider with an absolute file path pattern matching files in that database's source archive. Alternatively it might be the case that registering a broadly applicable DefinitionProvider isn't harmful as long as it simply returns zero results when another DefinitionProvider also exists.

[jcreedcmu]

Actually, wait, no, here is one concrete reason why not to make a DefinitionProvider that matches every file everywhere: iiuc, the bare fact of it being registered means "Jump to Definition" will always be in the context menu, even when wildly inappropriate, like in arbitrary text files.

[aeisenberg]

Since we are now actively zipping sources for zipped databases, is this still something worth working on?

[jcreedcmu]

Assume you mean 'actively zipping sources for unzipped-source-archive databases, when they are downloaded from within the extension'. I think this issue is still worth keeping open to the extent that it's possible for someone to download a database from lgtm themselves and try to work with it in the extension.

[aeisenberg]

Sure. Let's keep this open, though the priority is lower now.

[adityasharad]

Instead of writing new definition providers, could we approach this by prompting the user when they first attempt jump-to-def, and then zipping up the source archive for the database they chose? That would allow us to reuse the work you've already done to support databases downloaded from LGTM within the extension. I think the two situations are similar enough that we should handle them in similar ways.

[aeisenberg]

The difference is that if we zip sources in an unzipped database, that's a change to the user's files outside of the workspace and the extension's control. In general, it's not good practice for extensions to reach into the file system and make changes unless the user explicitly accepts them. We don't know if the user is relying on this structure in some way.

This case is a little different because:

1. An unzipped source location is basically deprecated
2. We are only updating the file system specific to Code QL.
3. Also, we can choose to zip non-destructively if we avoid deleting the unzipped source folder.

Though, waiting until the user issues a Jump-to-def command to do the zipping will be difficult since we'd need to do a bunch of cleanup and workspace changes on the fly (like close the source files and re-open then from the zipped location) and update workspace folders). It might be easiest to do this on workspace startup or when the workspace is added.

[adityasharad]

All wise points. Your suggestions (ask the user explicitly to make sure they are not relying on the structure, keeping the unzipped folder, and doing this operation at the time of loading the workspace / adding the database) reduce surprises for the user while hopefully still reducing the amount of new work that you would need to do.

[aeisenberg]

Summarizing the points above. Here is the approach we can take:

1. For existing databases with unzipped sources, prompt on startup to zip.
2. For new databases, prompt when adding.

Some more things to be aware of:

1. Should keep the unzipped folder
2. Existing editors coming from unzipped source locations should be closed (only necessary when prompted at startup)
3. Remove unzipped source folders from workspace
4. Add a setting for this so that users are never prompted.
5. Create a command so that users can forcibly unzip whenever they want.