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

Add --devtools argument #1849

Merged
merged 1 commit into from
Jun 9, 2023
Merged

Conversation

JeremyRand
Copy link
Contributor

Currently, chaiNNer assumes that Electron's DevTools should be automatically opened on launch if and only if an external Electron binary is in use. While this assumption holds quite often (an external Electron binary is used with npm run start), it is not always correct. For example, users might want to use a distro-packaged Electron binary on platforms (e.g. Linux/ppc64le) where chaiNNer does not distribute binaries.

This commit replaces the app.isPackaged condition with an explicit --devtools command-line flag, and tweaks package.json to provide this flag in the situations where an external Electron binary would normally have been used (e.g. npm run start), thus fixing the issue.

As a side benefit, users of official chaiNNer binaries who, for whatever reason, want to automatically launch Electron's DevTools, can now do so by passing the new command-line flag.

@JeremyRand
Copy link
Contributor Author

Good catch by the linter, will fix the warning ASAP.

Currently, chaiNNer assumes that Electron's DevTools should be
automatically opened on launch if and only if an external Electron
binary is in use. While this assumption holds quite often (an external
Electron binary is used with "npm run start"), it is not always correct.
For example, users might want to use a distro-packaged Electron binary
on platforms (e.g. Linux/ppc64le) where chaiNNer does not distribute
binaries.

This commit replaces the app.isPackaged condition with an explicit
--devtools command-line flag, and tweaks package.json to provide this
flag in the situations where an external Electron binary would normally
have been used (e.g. "npm run start"), thus fixing the issue.

As a side benefit, users of official chaiNNer binaries who, for whatever
reason, want to automatically launch Electron's DevTools, can now do so
by passing the new command-line flag.
@JeremyRand
Copy link
Contributor Author

There we go, CI is passing now.

@JeremyRand
Copy link
Contributor Author

As a side benefit, users of official chaiNNer binaries who, for whatever reason, want to automatically launch Electron's DevTools, can now do so by passing the new command-line flag.

On further testing, it looks like the flag only works when an external Electron binary is used; the DevTools pane doesn't appear when using chaiNNer's bundled Electron binary regardless of the flag. I think this PR is still clearly an improvement (e.g. people can download a chaiNNer binary zip, and use it with an external Electron binary to get the pane without needing to clone Git), and I'm not sure how best to deal with that edge case beyond that. Maybe just add a note to the help text for that flag so that affected users know why it doesn't work?

@RunDevelopment
Copy link
Member

Thanks for the PR @JeremyRand!

Code looks good, but I still have to test it.

the DevTools pane doesn't appear when using chaiNNer's bundled Electron binary regardless of the flag

Interesting. Is DevTools completely removed/non-functional or does it just fail to open? In any case, we should add this fact to the description of the devtools flag.

@JeremyRand
Copy link
Contributor Author

Interesting. Is DevTools completely removed/non-functional or does it just fail to open? In any case, we should add this fact to the description of the devtools flag.

Hmm, now I can't reproduce that issue anymore; DevTools works fine for me now with chaiNNer's bundled Electron binary. Probably I did something wrong earlier, might have been an artifact of running it in a VM without enough RAM. Carry on, I guess.

@joeyballentine joeyballentine merged commit 1c49b2c into chaiNNer-org:main Jun 9, 2023
@JeremyRand JeremyRand deleted the dev-tools branch December 5, 2023 23:04
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.

3 participants