Skip to content
This repository has been archived by the owner on Feb 16, 2024. It is now read-only.

The ability to keeping the console on Windows OS #47

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

NightMan-1
Copy link

I need console in my app, this option allow keep it, may be this will be interesting other people's :)

Copy link
Owner

@asticode asticode left a comment

Choose a reason for hiding this comment

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

Good idea!

A few minor changes though.

Cheers

@@ -138,6 +141,7 @@ type Bundler struct {
pathWorkingDirectory string
pathManifest string
resourcesAdapters []ConfigurationResourcesAdapter
KeepWindowsConsole bool
Copy link
Owner

Choose a reason for hiding this comment

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

Could you make the first letter a lower case letter (keepWindowsConsole) so that it's not exported?

@@ -360,7 +365,7 @@ func (b *Bundler) bundle(e ConfigurationEnvironment) (err error) {
`"main.BuiltAt=` + time.Now().String() + `"`,
},
}
if e.OS == "windows" {
if e.OS == "windows" && b.KeepWindowsConsole != true {
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a reason why you don't use !b.KeepWindowsConsole instead of b.KeepWindowsConsole != true since b.KeepWindowsConsole is a bool?

Copy link
Author

Choose a reason for hiding this comment

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

Hi! Not have any reasons, just learning the language and still getting confused :)

Copy link
Owner

Choose a reason for hiding this comment

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

OK could you replace it with !b.KeepWindowsConsole then ?

@marccardinal
Copy link

I stumbled upon this when looking for a solution to keep the console open on windows, let me know if I can help with testing or something :)

@asticode
Copy link
Owner

Thanks for proposing your help @marccardinal.

Right now I'm waiting for @NightMan-1 to make the few minor changes I requested, but once the PR is merged I'd welcome testing it in other conditions.

Cheers

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants