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

set default value of colorTheme and iconTheme preference #8381

Merged
merged 1 commit into from
Aug 17, 2020

Conversation

akosyakov
Copy link
Member

What it does

fix #8374: make sure that color and icon themes are having default values.

Python extension reads these values to generate theme for a webview. If they are missing then dark is used what is happening here #8374 (comment)

How to test

  • install python extension from the open vsxk
  • create a new notebook
  • check that highlighting in notebook matches to the current theme
    • there can be a bit of a delay, python extension needs to create json file with theme and load it in the webview to apply
  • check that logs don't have errors about undefined theme
  • change the heme, check that theme is eventually is applied
    • again here can be delay required on theme regeneration by the extension
  • reload the page and observe that loading screen as well as shell never flickes between default dark theme and one which you selected before
  • try again to create a notebook and check that theme is proper

Review checklist

Reminder for reviewers

@akosyakov akosyakov added python issues related to the python language / extension theming issues related to theming vscode issues related to VSCode compatibility labels Aug 14, 2020
@akosyakov akosyakov force-pushed the tom-shan-preference-colorTheme branch 3 times, most recently from df0d789 to 2b237c2 Compare August 14, 2020 07:58
@akosyakov
Copy link
Member Author

tests are failing now, I will have a look

@akosyakov akosyakov force-pushed the tom-shan-preference-colorTheme branch from 2b237c2 to abfbc87 Compare August 14, 2020 12:47
@akosyakov
Copy link
Member Author

@kittaakos I've addressed your comments and fixed tests.

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I'll perform a functional review shortly.

dev-packages/application-package/src/application-props.ts Outdated Show resolved Hide resolved
@kittaakos
Copy link
Contributor

I've addressed your comments and fixed tests.

Nice, I checked the code, it looks good, now I am going to verify it with the Python extension as suggested.

@kittaakos
Copy link
Contributor

I see a lot of errors in the console:

f JSON input
    at JSON.parse (<anonymous>)
    at /Users/akos.kitta/git/theia/node_modules/jsonfile/index.js:33:18
    at /Users/akos.kitta/git/theia/node_modules/graceful-fs/graceful-fs.js:115:16
    at FSReqCallback.readFileAfterClose [as oncomplete] (internal/fs/read_file_context.js:61:3)
root ERROR Failed to parse data from " /Users/akos.kitta/.theia/plugin-storage/global-state.json ". Reason: SyntaxError: /Users/akos.kitta/.theia/plugin-storage/global-state.json: Unexpected end of JSON input
    at JSON.parse (<anonymous>)
    at /Users/akos.kitta/git/theia/node_modules/jsonfile/index.js:33:18
    at /Users/akos.kitta/git/theia/node_modules/graceful-fs/graceful-fs.js:115:16
    at FSReqCallback.readFileAfterClose [as oncomplete] (internal/fs/read_file_context.js:61:3)
root ERROR Failed to parse data from " /Users/akos.kitta/.theia/plugin-storage/global-state.json ". Reason: SyntaxError: /Users/akos.kitta/.theia/plugin-storage/global-state.json: Unexpected end of JSON input

I am not sure if it is related to the theme changes or nor; probably not.

@kittaakos
Copy link
Contributor

I evaluated (Ctrl+Enter) println('123') and got this as an error. It's OK to see the error, I do not have the required Python 3.x interpreter, but it's not good that I cannot copy the stack trace. 😕

Screen Shot 2020-08-14 at 15 11 32

@kittaakos kittaakos self-requested a review August 14, 2020 13:22
Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

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

I have verified it locally with the electron example. The changeset looks good, the app works well. Thank you! 👍

@akosyakov
Copy link
Member Author

@kittaakos I don't think an exception related to changes. I've opened a follow-up issue: #8384

Also-by: tom-shan <swt0008411@163.com>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov force-pushed the tom-shan-preference-colorTheme branch from abfbc87 to d12c8e5 Compare August 14, 2020 13:46
@akosyakov akosyakov merged commit f58de79 into master Aug 17, 2020
@akosyakov akosyakov deleted the tom-shan-preference-colorTheme branch August 17, 2020 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python issues related to the python language / extension theming issues related to theming vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

theming of embedded editors in python notebook sometimes off
3 participants