You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2021/01/05 12:07:05 UTC
[GitHub] [cordova-electron] zoltan-mihalyi opened a new pull request #180: Fix Electron window is hidden in windows, stdio ignored
zoltan-mihalyi opened a new pull request #180:
URL: https://github.com/apache/cordova-electron/pull/180
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
### Platforms affected
- Windows (electron window is hidden)
- others (stdio is ignored)
### Motivation and Context
- On windows platform using nodejs 11+, the electron window does not appear.
- Standard output from electron is ignored
### Description
- `windowsHide: false` makes the electron window appear on windows.
- `stdio: 'inherit'` makes the output of electron appear in the console.
### Testing
I manually tested my changes on windows.
### Checklist
- [x] I've run the tests to see all new and existing tests pass
- [x] I added automated test coverage as appropriate for this change
- [x] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
- [x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
- [x] I've updated the documentation if necessary
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-electron] erisu commented on pull request #180: fix(windows): electron window not displaying
Posted by GitBox <gi...@apache.org>.
erisu commented on pull request #180:
URL: https://github.com/apache/cordova-electron/pull/180#issuecomment-756649074
@zoltan-mihalyi I just squashed merged the PR so you can update your fork copy and use the main branch to create the second PR from.
I will review & merge the second PR as soon as I see it ready & finishes running the tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-electron] zoltan-mihalyi commented on pull request #180: fix(windows): electron window not displaying
Posted by GitBox <gi...@apache.org>.
zoltan-mihalyi commented on pull request #180:
URL: https://github.com/apache/cordova-electron/pull/180#issuecomment-756642772
@erisu The first PR is done, I will create the second one after this is finished.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-electron] erisu commented on pull request #180: Fix Electron window is hidden in windows, stdio ignored
Posted by GitBox <gi...@apache.org>.
erisu commented on pull request #180:
URL: https://github.com/apache/cordova-electron/pull/180#issuecomment-756621967
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-electron] erisu merged pull request #180: fix(windows): electron window not displaying
Posted by GitBox <gi...@apache.org>.
erisu merged pull request #180:
URL: https://github.com/apache/cordova-electron/pull/180
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-electron] erisu commented on pull request #180: Fix Electron window is hidden in windows, stdio ignored
Posted by GitBox <gi...@apache.org>.
erisu commented on pull request #180:
URL: https://github.com/apache/cordova-electron/pull/180#issuecomment-756621967
Hey @zoltan-mihalyi,
Thank you for the PR.
I have configured my Windows environment and tested the before and after case to confirm the original issue and fix.
I would like to actually split this PR into two PRs. I know this PR is small and only changes one line, but IMO this PR is doing two things, a fix and a feature.
You can make the second PR based from the first. You or I can rebasae after we merge in the first fix PR.
### First PR (Fix)
Lets convert this PR into a fix only PR.
* Change the PR title into something like this:
> fix(windows): electron window not displaying
* Update the description to only apply for this fix.
* Update the code to only apply the fix.
### Second PR (Feature)
IMO, adding the `stdio` flag is more of a feature request and not apart of the fix.
Lets make a new PR for this feature request.
* Create a PR title into something like this:
> feat: display electron’s process stdio in terminal
* Set the description explaining the new feature, etc.
* Add new feature code.
### Overall
The changes are good and works..
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-electron] zoltan-mihalyi commented on pull request #180: fix(windows): electron window not displaying
Posted by GitBox <gi...@apache.org>.
zoltan-mihalyi commented on pull request #180:
URL: https://github.com/apache/cordova-electron/pull/180#issuecomment-756642772
@erisu The first PR is done, I will create the second one after this is finished.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-electron] erisu merged pull request #180: fix(windows): electron window not displaying
Posted by GitBox <gi...@apache.org>.
erisu merged pull request #180:
URL: https://github.com/apache/cordova-electron/pull/180
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-electron] miroslavvojtus commented on pull request #180: fix(windows): electron window not displaying
Posted by GitBox <gi...@apache.org>.
miroslavvojtus commented on pull request #180:
URL: https://github.com/apache/cordova-electron/pull/180#issuecomment-834087548
We have the same problem.
When we can expect the next version?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org