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

Continue on for comments doesn't work sometimes #194288

Closed
alexr00 opened this issue Sep 27, 2023 · 8 comments
Closed

Continue on for comments doesn't work sometimes #194288

alexr00 opened this issue Sep 27, 2023 · 8 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug comments Comments Provider/Widget/Panel issues verified Verification succeeded
Milestone

Comments

@alexr00
Copy link
Member

alexr00 commented Sep 27, 2023

Testing #193104

To repro, follow the steps in the above test.

@alexr00 alexr00 added bug Issue identified by VS Code Team member as probable bug comments Comments Provider/Widget/Panel issues labels Sep 27, 2023
@alexr00 alexr00 added this to the September 2023 milestone Sep 27, 2023
@alexr00 alexr00 self-assigned this Sep 27, 2023
@alexr00
Copy link
Member Author

alexr00 commented Sep 27, 2023

@joyceerhl I'm seeing an unexpected URI in the object that I get from the storage service when I try to restore comments:

image

The two with the long URI authority restore, because that URI matches the URI that of the files "locally" (it's actually GitHub Repositories, but it's the machine I'm trying to restore on).

@alexr00
Copy link
Member Author

alexr00 commented Sep 27, 2023

On the machine I'm storing the comment from, the URI doesn't have that long authority:
image

@alexr00
Copy link
Member Author

alexr00 commented Sep 27, 2023

I'm seeing this URI difference when trying to restore across two actual different machines using VS Code insiders (desktop) on both.

@alexr00 alexr00 changed the title Continue on for comments doesn't work Continue on for comments doesn't work sometimes Sep 27, 2023
@joyceerhl
Copy link
Collaborator

Hm, I cannot repro because comments aren't showing up at all for me in web https://insiders.vscode.dev/github/microsoft/vscode/pull/193812
image

@alexr00
Copy link
Member Author

alexr00 commented Sep 28, 2023

@joyceerhl I'm seeing comments show up as expected for that PR. Can you share output from GitHub Pull Request and see if there are any errors in the console?

@joyceerhl
Copy link
Collaborator

OK, comments show up properly for that PR now. I'm still running into some issues though:

  1. I had to enable comments.experimentalContinueOn first, otherwise no state was getting stored:
  2. I see the following comment state getting stored for the above PR, the first entry seems like stale state since it's not coming from the current workspace. Do you have insight into where it comes from?
[
  {
    owner: 'Hide launcher on Run and Debug title bar when debugging-GitHub.vscode-pull-request-github',
    uri: {
      '$mid': 1,
      fsPath: 'c:\\Users\\huer\\Source\\vscode\\src\\vs\\workbench\\contrib\\debug\\browser\\debug.contribution.ts',        
      _sep: 1,
      external: 'file:///c%3A/Users/huer/Source/vscode/src/vs/workbench/contrib/debug/browser/debug.contribution.ts',       
      path: '/c:/Users/huer/Source/vscode/src/vs/workbench/contrib/debug/browser/debug.contribution.ts',
      scheme: 'file'
    },
    range: {
      startLineNumber: 455,
      startColumn: 1,
      endLineNumber: 455,
      endColumn: 1
    },
    body: 'foo'
  },
  {
    owner: 'Hide launcher on Run and Debug title bar when debugging-GitHub.vscode-pull-request-github',
    uri: {
      '$mid': 1,
      fsPath: '\\microsoft\\vscode\\src\\vs\\workbench\\contrib\\debug\\browser\\debug.contribution.ts',
      _sep: 1,
      external: 'vscode-vfs://github%2B7b2276223a312c22726566223a7b2274797065223a332c226964223a22313933383132227d7d/microsoft/vscode/src/vs/workbench/contrib/debug/browser/debug.contribution.ts',
      path: '/microsoft/vscode/src/vs/workbench/contrib/debug/browser/debug.contribution.ts',
      scheme: 'vscode-vfs',
      authority: 'github+7b2276223a312c22726566223a7b2274797065223a332c226964223a22313933383132227d7d'
    },
    range: {
      startLineNumber: 453,
      startColumn: 1,
      endLineNumber: 453,
      endColumn: 1
    },
    body: 'foooo'
  }
]
  1. If I set a breakpoint here I don't see it get hit after we call storeAll with the restored value, i.e. the onDidChangeValue event listener that comments registers doesn't fire, so it doesn't update with the restored comments data, will keep investigating why that might be the case

@alexr00
Copy link
Member Author

alexr00 commented Oct 5, 2023

I had to enable comments.experimentalContinueOn first, otherwise no state was getting stored

This is intentional. During testing, we couldn't get a good test of the feature so I put it behind a hidden setting so that we didn't accumulate a bunch of junk state for 1.83 for users.

I see the following comment state getting stored for the above PR, the first entry seems like stale state since it's not coming from the current workspace. Do you have insight into where it comes from?

I'm not sure what that could be, but the logging I added yesterday should help shed some light on that. If you set the log level of "Window" to "debug", then you should get lines like "Comments: URIs of continue on comments..." for when we store and retrieve comments to and from the storage service.

@alexr00
Copy link
Member Author

alexr00 commented Oct 10, 2023

@joyceerhl I found that testing across two desktop installs worked extremely reliably. Before, I was testing across desktop and insiders.vscode.dev, and it was unreliable (as described in step 3) so I didn't notice all the bugs that I had. All the bugs I could find I fixed in #195260.

For testing, I'll update the TPI to ask that testers use two desktop installs of VS Code.

@alexr00 alexr00 closed this as completed Oct 10, 2023
alexr00 added a commit that referenced this issue Oct 10, 2023
* Various continue on comments fixes
Part of #194288

* Fix continue-on reply being restored before root comment
Fix duplicate continue-on comments

* Include monaco.d.ts changes
Alex0007 pushed a commit to Alex0007/vscode that referenced this issue Oct 26, 2023
Alex0007 pushed a commit to Alex0007/vscode that referenced this issue Oct 26, 2023
* Various continue on comments fixes
Part of microsoft#194288

* Fix continue-on reply being restored before root comment
Fix duplicate continue-on comments

* Include monaco.d.ts changes
@connor4312 connor4312 added the verified Verification succeeded label Oct 27, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug comments Comments Provider/Widget/Panel issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants