Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline evaluation results no longer display in 2.0.216 #1332

Closed
S4G4R opened this issue Oct 12, 2021 · 25 comments
Closed

Inline evaluation results no longer display in 2.0.216 #1332

S4G4R opened this issue Oct 12, 2021 · 25 comments
Labels
bug Something isn't working repl

Comments

@S4G4R
Copy link
Contributor

S4G4R commented Oct 12, 2021

Version: 1.61.0 (user setup)
Commit: ee8c7def80afc00dd6e593ef12f37756d8f504ea
Date: 2021-10-07T18:13:09.652Z
Electron: 13.5.1
Chrome: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Windows_NT x64 10.0.19043

@PEZ PEZ added bug Something isn't working repl labels Oct 12, 2021
@PEZ
Copy link
Collaborator

PEZ commented Oct 12, 2021

Thanks for reporting! Are you using GitLense perhaps?

@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 12, 2021

I am using GitLens, yes. But disabling it doesn't seem to fix the bug.

@PEZ
Copy link
Collaborator

PEZ commented Oct 12, 2021

Thanks for trying that. Very strange. Do you have some other extension that annotates behind the line endings?

I suspect it is a race condition between Calva and some other extension, but I'm really unsure about it, because I can have GitLense line blames on w/o conflict with inline results.

BTW, anyone reading this and experiencing this bug. Run with Calva v2.0.215 for now. In the Extension pane, click the cogwheel in the Calva entry:

image

@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 12, 2021

I disabled all my other extensions except Calva, but it still didn't fix it. Here are the ones that I am actively using :

  • Calva
  • GitHub Pull Requests and Issues
  • GitLens
  • PostgreSQL
  • Remote-WSL
  • One Dark Pro Theme (Tried changing this too)

@PEZ
Copy link
Collaborator

PEZ commented Oct 12, 2021

Hmmm. Does it work with v2.0.215? (I want to rule out that this is a freak coincidence with me changing how inline display is done. 😄)

@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 12, 2021

It works fine with 2.0.215, it's what I've been using since I found this bug.

@PEZ
Copy link
Collaborator

PEZ commented Oct 12, 2021

Thanks for updating about that. Since I can't reproduce the error, I might ask you for help debugging this. Would you be up for that? It's pretty easy to setup the dev environment, and it is detailed on the wiki.

@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 12, 2021

Sure, no problem. I will have a look at the wiki and see if I can setup the environment.

@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 13, 2021

@PEZ I have the dev environment setup. Let me know if there is anything specific I can do.

@PEZ
Copy link
Collaborator

PEZ commented Oct 13, 2021

That's awesome! The change that messes it up for you lives in src/providers/annotations.ts, in the function evaluated. You should be able to confirm that changing the after: keys to :before makes things work again.

I am curious about the difference in the DOM when it works and when it doesn't. If you open the developers tools (available from both the command palette and the Help menu) and inspect around the end of the line where the results are (or are not) displayed, maybe you can find a clue.

That's all I can think up right now. It is a very strange error.

@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 14, 2021

Changing after: to before: (I think thats what you meant instead of :before?), doesn't seem to do much.
Here is the error and stack trace that I get in the debug console though when I try to evaluate (+ 1 1) :

rejected promise not handled within 1 second: TypeError: contentText.replaceAll is not a function
/home/sagar/.vscode-server/bin/ee8c7def80afc00dd6e593ef12f37756d8f504ea/out/bootstrap-fork.js:5
stack trace: TypeError: contentText.replaceAll is not a function
    at evaluated (/home/sagar/calva/src/providers/annotations.ts:57:42)
    at Object.decorateResults (/home/sagar/calva/src/providers/annotations.ts:126:22)
    at /home/sagar/calva/src/evaluate.ts:95:45
    at Generator.next (<anonymous>)
    at /home/sagar/calva/out/evaluate.js:8:71
    at new Promise (<anonymous>)
    at __awaiter (/home/sagar/calva/out/evaluate.js:4:12)
    at /home/sagar/calva/src/evaluate.ts:79:65
    at /home/sagar/calva/src/results-output/results-doc.ts:297:25
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

@PEZ
Copy link
Collaborator

PEZ commented Oct 14, 2021

Ah, so it was that other change. 😄

This is even more strange, though. Why wouldn't String.replaceAll() be there... Can you catch it in the debugger and play around with a bit? Is contextText even a string?

@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 14, 2021

All seems fine :

console.log(contentText)
console.log(typeof contentText)

Gives me :

=> 2
string

@PEZ
Copy link
Collaborator

PEZ commented Oct 14, 2021

So it is a string... please also check if any of these work:

contentText.replaceAll(' ', "\u00a0")
contentText.replace(/ /g, "\u00a0")

@PEZ
Copy link
Collaborator

PEZ commented Oct 14, 2021

Debug tips: Instead of console.log you can use LogPoints. Right-click in the gutter to add one. Something like {cotentText} : {typeof contentText} in this case. Just good to know, in case you didn't. Also, I often find placing a break point and using the debug console is nice in aREPL-ish way.

@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 14, 2021

contentText.replaceAll(' ', "\u00a0")
This one throws the same error as before.
contentText.replace(/ /g, "\u00a0")
This one works!

@PEZ
Copy link
Collaborator

PEZ commented Oct 14, 2021

Wow, that's so strange! Why wouldn't replaceAll be there... This probably means that a few other Calva features do not work for you. ... Hmmm, actually we only use replaceAll in one other feature: Custom command snippets.

I wonder if babel can be configured to fix it for us... But I also wonder about the root cause. ... Ah, a comment on this SO answer says it was added in nodejs 15. https://stackoverflow.com/a/1145525/44639 Which VS Code version are you on?

@PEZ
Copy link
Collaborator

PEZ commented Oct 14, 2021

1.61.0, it's the Version in the OP start, right? I wonder what (user setup) means...

@PEZ
Copy link
Collaborator

PEZ commented Oct 14, 2021

This is from the about box on my VS Code:

Version: 1.61.0
Commit: ee8c7def80afc00dd6e593ef12f37756d8f504ea
Date: 2021-10-07T18:11:58.853Z
Electron: 13.5.1
Chrome: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Darwin x64 20.6.0

Putting yours here for easy comparison:

Version: 1.61.0 (user setup)
Commit: ee8c7def80afc00dd6e593ef12f37756d8f504ea
Date: 2021-10-07T18:13:09.652Z
Electron: 13.5.1
Chrome: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Windows_NT x64 10.0.19043

@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 14, 2021

It seems there are options for "User Installer" and "System Installer", and I may have chosen the former. Let me try the System Installer.
https://code.visualstudio.com/download

@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 14, 2021

Well, all that has changed is instead of "user setup" it nows says "system setup" 😅. I think it is a Windows thing, and I noticed you are using Mac.
The bug also still persists.

@PEZ
Copy link
Collaborator

PEZ commented Oct 14, 2021

Now tested on my Windows machine and it works there.

Version: 1.61.0 (user setup)
Commit: ee8c7def80afc00dd6e593ef12f37756d8f504ea
Date: 2021-10-07T18:13:09.652Z
Electron: 13.5.1
Chrome: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Windows_NT x64 10.0.19041

This goes deeper. 😄

We can of course use replace where we now use replaceAll. If you want to supply a PR, I'd be happy to accept. But we should then also create an issue about the root cause not being found. And we need to find it, because other bugs are lurking and/or waiting to happen.

@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 14, 2021

I'd be happy to provide a PR. Should I create a separate issue to find the root cause?

S4G4R added a commit to S4G4R/calva that referenced this issue Oct 14, 2021
@PEZ
Copy link
Collaborator

PEZ commented Oct 14, 2021

Awesome. Yes, please file an issue about replaceAll and link it here. Then in the PR you can put Fixes #1332 and something like Workaround for #<new issue> That'll give us a good trail, I think.

Please also place the same workaround in src/custom-snippets.ts. 🙏

S4G4R added a commit to S4G4R/calva that referenced this issue Oct 14, 2021
@S4G4R
Copy link
Contributor Author

S4G4R commented Oct 14, 2021

Linked : #1339

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working repl
Projects
None yet
Development

No branches or pull requests

2 participants