Remove unnecessary code lens refresh

[jribbink]

The codelens was being tricked into refreshing using document modification every time the emulator state changed. This is ultimately unnecessary, grandfathered behaviour as the language server is restarted every time the emulator state changes (so code lens & other changes will be reflected automatically)

Closes #218

---

For contributor use:

• Targeted PR against master branch
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work
• Code follows the standards mentioned here
• Updated relevant documentation
• Re-reviewed Files changed in the Github PR explorer
• Added appropriate labels

[jribbink]

Put this as a draft because I'm not totally sure if it is necessary to refresh code lenses at all considering that we are restarting the language server every time that the emulator state changes. (i.e. the code lenses refresh anyways because we restart the language server).

I'm assuming we can probably just ditch this functionality altogether.

[sideninja]

The first thing is to remove the file changes to trigger codeless refresh, this was now done every time you opened a file, and it resulted in a weird behavior asking you whether you want to save a file or not even though you just opened it to view it.

The other part is, do we need to keep manual codelens refresh, I believe we had reasons to do that in the past (and since it's a while since I worked on this codebase I forgot what all the reasons were), but you can try disabling this and see if it behaves correctly. I would test couple of cases:

• open a file, restart the emulator, see if the codelensses work as well as autocompletions
• open a file (contract), deploy it, add another file (contract) import that first deployed contract, try to access public variables on that imported contract (should suggest correctly), then go back to first contract and add a new member and redeploy it, then go back to that second contract importing the first one and see if the autocomplete includes the newly added member (I believe this was one of the important reasons for manual refresh, because the second file didn't really change, thus not requesting the new codelensses), but also it might be that we need more than just codelenss request, since the editing a file triggered more actions, so it might be we need to trigger additional actions to get the result we need, in autocompletion case that is connected to textDocument/didChange I believe.

In short, try out how it works with removing automatic file edits and not calling the refresh. Be careful on imports, autocompletions, and codelensses when you do changes (like adding another argument to a script).

[jribbink]

The first thing is to remove the file changes to trigger codeless refresh, this was now done every time you opened a file, and it resulted in a weird behavior asking you whether you want to save a file or not even though you just opened it to view it.

The other part is, do we need to keep manual codelens refresh, I believe we had reasons to do that in the past (and since it's a while since I worked on this codebase I forgot what all the reasons were), but you can try disabling this and see if it behaves correctly. I would test couple of cases:

• open a file, restart the emulator, see if the codelensses work as well as autocompletions
• open a file (contract), deploy it, add another file (contract) import that first deployed contract, try to access public variables on that imported contract (should suggest correctly), then go back to first contract and add a new member and redeploy it, then go back to that second contract importing the first one and see if the autocomplete includes the newly added member (I believe this was one of the important reasons for manual refresh, because the second file didn't really change, thus not requesting the new codelensses), but also it might be that we need more than just codelenss request, since the editing a file triggered more actions, so it might be we need to trigger additional actions to get the result we need, in autocompletion case that is connected to textDocument/didChange I believe.

In short, try out how it works with removing automatic file edits and not calling the refresh. Be careful on imports, autocompletions, and codelensses when you do changes (like adding another argument to a script).

I've just tested everything and it all performs the same when I remove the CodeLens refresh.

To be sure that we didn't need it I looked into the history of the function and it was created back in 2021 at a time when emulator state changes did not necessarily imply a reload of the language server. When it was created, there were times that the emulator would turn on/off, but the language server would never be restarted.

It just looks like unnecessary grandfathered code that we can get rid of.

[jribbink]

just changed this to a typed request handler since it's probably better practice

[sideninja]

The first thing is to remove the file changes to trigger codeless refresh, this was now done every time you opened a file, and it resulted in a weird behavior asking you whether you want to save a file or not even though you just opened it to view it.
The other part is, do we need to keep manual codelens refresh, I believe we had reasons to do that in the past (and since it's a while since I worked on this codebase I forgot what all the reasons were), but you can try disabling this and see if it behaves correctly. I would test couple of cases:

• open a file, restart the emulator, see if the codelensses work as well as autocompletions
• open a file (contract), deploy it, add another file (contract) import that first deployed contract, try to access public variables on that imported contract (should suggest correctly), then go back to first contract and add a new member and redeploy it, then go back to that second contract importing the first one and see if the autocomplete includes the newly added member (I believe this was one of the important reasons for manual refresh, because the second file didn't really change, thus not requesting the new codelensses), but also it might be that we need more than just codelenss request, since the editing a file triggered more actions, so it might be we need to trigger additional actions to get the result we need, in autocompletion case that is connected to textDocument/didChange I believe.

In short, try out how it works with removing automatic file edits and not calling the refresh. Be careful on imports, autocompletions, and codelensses when you do changes (like adding another argument to a script).

I've just tested everything and it all performs the same when I remove the CodeLens refresh.

To be sure that we didn't need it I looked into the history of the function and it was created back in 2021 at a time when emulator state changes did not necessarily imply a reload of the language server. When it was created, there were times that the emulator would turn on/off, but the language server would never be restarted.

It just looks like unnecessary grandfathered code that we can get rid of.

Nice, good job.

[sideninja]

@jribbink can you look at why the test fails?

[jribbink]

@jribbink can you look at why the test fails?

For Ubuntu - looks like flaky tests... ran it again twice and it passed, can see similar results in CI for other PRs. For Windows - looks like the tests are just broken and have been for the last few commits to master.

[sideninja]

@jribbink can you look at why the test fails?

For Ubuntu - looks like flaky tests... ran it again twice and it passed, can see similar results in CI for other PRs. For Windows - looks like the tests are just broken and have been for the last few commits to master.

I think this is worth investigating and fixing as a separate PR and issue.