Skip to content

Add a canary setting to avoid caching AST viewer queries#818

Merged
adityasharad merged 1 commit intomainfrom
aeisenberg/astviewer-nocache
Apr 1, 2021
Merged

Add a canary setting to avoid caching AST viewer queries#818
adityasharad merged 1 commit intomainfrom
aeisenberg/astviewer-nocache

Conversation

@aeisenberg
Copy link
Copy Markdown
Contributor

@aeisenberg aeisenberg commented Apr 1, 2021

When codeql library developers are working on PrintAST queries, it is
not easy to use the AST Viewer. The AST Viewer caches results so that
multiple calls to view the AST of the same file are nearly
instantaneous.

However, this breaks down if you are changing the actual queries that
perform AST viewing. In this case, you do not want the cache to be
active.

This commit adds an undocumented setting that prevents caching. To
enable, set:

"codeQL.isCanary": true,
"codeQL.astViewer.disableCache": true

Note that both settings must be true for this to work.

This behaviour and all canary behaviour should be documented somewhere.
I will add that later.

Any suggestions on where this should go would be appreciated.

Checklist

  • [n/a] CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • [n/a] Issues have been created for any UI or other user-facing changes made by this pull request.
  • [n/a] @github/docs-content-codeql has been cc'd in all issues for UI or other user-facing changes made by this pull request.

Copy link
Copy Markdown
Contributor

@adityasharad adityasharad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, one suggestion on setting groups. We could add an "advanced users" section to the public docs.

Comment thread extensions/ql-vscode/src/config.ts Outdated
/**
* Avoids caching in the AST viewer if the user is also a canary user.
*/
export const NO_CACHE_AST_VIEWER = new Setting('noCacheAstViewer', ROOT_SETTING);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to have an astViewer or viewAst parent setting, and then a noCache or disableCache underneath it? Not sure if we expect future AST viewer-related options.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that better. Made the change.

When codeql library developers are working on PrintAST queries, it is
not easy to use the AST Viewer. The AST Viewer caches results so that
multiple calls to view the AST of the same file are nearly
instantaneous.

However, this breaks down if you are changing the actual queries that
perform AST viewing. In this case, you do not want the cache to be
active.

This commit adds an undocumented setting that prevents caching. To
enable, set:

```
"codeQL.isCanary": true,
"codeQL.astViewer.disableCache": true
```

Note that *both* settings must be true for this to work.

This behaviour and all canary behaviour should be documented somewhere.
I will add that later.
@aeisenberg aeisenberg force-pushed the aeisenberg/astviewer-nocache branch from f80ae38 to 3f52e25 Compare April 1, 2021 21:05
@aeisenberg
Copy link
Copy Markdown
Contributor Author

I'll document it internally.

@adityasharad adityasharad merged commit c082a38 into main Apr 1, 2021
@adityasharad adityasharad deleted the aeisenberg/astviewer-nocache branch April 1, 2021 21:12
aofaof0907 pushed a commit to aofaof0907/vscode-codeql that referenced this pull request Jul 27, 2021
When codeql library developers are working on PrintAST queries, it is
not easy to use the AST Viewer. The AST Viewer caches results so that
multiple calls to view the AST of the same file are nearly
instantaneous.

However, this breaks down if you are changing the actual queries that
perform AST viewing. In this case, you do not want the cache to be
active.

This commit adds an undocumented setting that prevents caching. To
enable, set:

```
"codeQL.isCanary": true,
"codeQL.astViewer.disableCache": true
```

Note that *both* settings must be true for this to work.

This behaviour and all canary behaviour should be documented somewhere.
I will add that later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants