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

9012-updating-userstorageuri-scheme #9049

Conversation

danarad05
Copy link
Contributor

@danarad05 danarad05 commented Feb 10, 2021

scheme was changed from 'user_storage' to 'user-storage' as '_' is not a valid char in scheme (according to RFC 3986)

What it does

Fixes second problem in #9012 where JDTUtils threw exception when receiving file changed event whose URI scheme was user_storage -which is an illegal scheme name (contains underscore).
Therefore changed user_storage to user-storage scheme name.

For first problem in #9012 see #9048

How to test

  1. Pre-requisite: 9012-registerTerminalLinkProvider-stub: for vscode-java-debug #9048 is merged
  2. Open in Theia java project
  3. After load you can run or debug project
    image

image

Note:
for better behavior change this user setting to "internalConsole" (instead of "integratedTerminal"):
"java.debug.settings.console": "internalConsole".

Review checklist

Reminder for reviewers

@colin-grant-work
Copy link
Contributor

colin-grant-work commented Feb 12, 2021

It looks like the only way to address the problem. I've tested and confirmed that user storage is still being found correctly - preferences etc. are read and updated as expected. It looks like perhaps a sign-off is missing in the commit message (I believe that's why the Eclipse Foundation ECA check is failing), and the message itself is a bit long for a single line, but otherwise, this looks good. Hopefully there aren't too many explicit references to this URI in adopter code.

@danarad05 danarad05 force-pushed the 9012-updating-userstorageuri-scheme branch from 5dd3cb5 to a81b7b6 Compare February 14, 2021 14:19
@danarad05 danarad05 changed the title 9012-updating-userstorageuri-scheme: scheme was changed from 'user_st… 9012-updating-userstorageuri-scheme Feb 14, 2021
scheme was changed from 'user_storage' to 'user-storage' as '\_' is not a valid char in scheme (according to [RFC 3986](https://tools.ietf.org/html/rfc3986#page-17))

Signed-off-by: Dan Arad <[email protected]>
@danarad05 danarad05 force-pushed the 9012-updating-userstorageuri-scheme branch from a81b7b6 to 8d53a41 Compare February 14, 2021 14:23
@danarad05
Copy link
Contributor Author

It looks like the only way to address the problem. I've tested and confirmed that user storage is still being found correctly - preferences etc. are read and updated as expected. It looks like perhaps a sign-off is missing in the commit message (I believe that's why the Eclipse Foundation ECA check is failing), and the message itself is a bit long for a single line, but otherwise, this looks good. Hopefully there aren't too many explicit references to this URI in adopter code.

Thanks @colin-grant-work for review. Fixed signoff and long commit message.
Thanks

@amiramw
Copy link
Member

amiramw commented Feb 23, 2021

I would like to merge this before the release if there is no objection

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.

5 participants