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

Added Commandline Options #235

Merged
merged 17 commits into from Mar 7, 2018
Merged

Added Commandline Options #235

merged 17 commits into from Mar 7, 2018

Conversation

ghost
Copy link

@ghost ghost commented Mar 1, 2018

Added Commandline Options "--disable-hardware-acceleration" & "--force-devtools".

With this commit, hardware acceleration would be on as default. Linux User with problems can use the option "--disable-hardware-acceleration" forr the workaround mentioned in #128.

I also added the "--force-devtools" option, so you dont have to build a special version for debugging the production packages.

I think these should be documented in the readme, any suggestions where?

@hql287
Copy link
Owner

hql287 commented Mar 2, 2018

@jens-t Thanks for the PR. 👍

Quick question: How do you know the exported app will take these argument (--force-devtools and --disable-hardware-acceleration)?

@ghost
Copy link
Author

ghost commented Mar 2, 2018

Quick question: How do you know the exported app will take these argument (--force-devtools and --disable-hardware-acceleration)?

I tested this with a special version, which logged some debug information to the devtools console. but you can test this in the following ways:

  • Dev-Server: electron . --force-devtools
  • Linux AppImage: ./Manta-1.1.2-x86_64.AppImage --force-devtools
  • Installed App (.deb): manta --force-devtools (works in the same way for macOS & win ...)
    ... and the devtools will start with the app.

@hql287
Copy link
Owner

hql287 commented Mar 2, 2018

Ok, then I think we should update the README as well. Maybe somewhere in the Development section. Also, if we disable the Hardware Acceleration, will the black box show up again?

@ghost
Copy link
Author

ghost commented Mar 2, 2018

Ok, then I think we should update the README as well. Maybe somewhere in the Development section.

ok. but i think it doesn't fit in the development section. it's more a user feature, and maybe more flags are following, so it maybe deserves its own section. i make a suggestion with a commit.

Also, if we enable the Hardware Acceleration, will the black box show up again?

well, this pull request is referenced in the old pull request of the user having the problem. so the user should know, that there is a flag for this now. i searched the issues and found no one else having this problem, but i can add a short info to the readme ... linking to electron/electron#4322, but i think that is to niche and specific for the readme. other users that eventually will have the problem will also find the pull request of @c0b41. and it is a bug from februar 2016, so it is possible that the black box might not show at all, not even for @c0b41.

@hql287
Copy link
Owner

hql287 commented Mar 2, 2018

@jens-t You're on Linux, right? Does the black box show up on your screen if you enable hardware acceleration?

@ghost
Copy link
Author

ghost commented Mar 2, 2018

Yes, thats right. And no, i dont have any problems with hwa on, but the app is broken for me with hwa off. The "black box problem" is an edge case, not a general problem for all linux users.

@hql287 hql287 added this to the Build 06 - v1.1.4 milestone Mar 2, 2018
@hql287 hql287 assigned ghost Mar 3, 2018
README.md Outdated
@@ -104,6 +104,10 @@ Windows 7 and later are supported

[More information](https://github.com/electron/electron/blob/master/docs/tutorial/supported-platforms.md).

### App Start-Options
Copy link
Owner

@hql287 hql287 Mar 5, 2018

Choose a reason for hiding this comment

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

I think it would be better if we could add some kind of explanation for why it's here and what it does. Or better yet, reference the issue(s) that led to this change. I think Linux users will appreciate it. 👍

Also, it seems a little too short to has its own section, maybe move it under Development?

Copy link
Author

@ghost ghost Mar 5, 2018

Choose a reason for hiding this comment

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

I think it would be better if we could add some kind of explanation for why it's here and what it does. Or better yet, reference the issue(s) that led to this change. I think Linux users will appreciate it.

Yes, thats a good idea. I make a proposal.

Also, it seems a little too short to has its own section, maybe move it under Development?

Is it really related to development? I also thought about adding a start option for a path to a custom theme, so maybe the section gets bigger in the future :) If it shouldn't be part of the readme, maybe it should get an own wiki page?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, I think it's related to Development because you'll only use those options for development purpose. But I think it deserves its own Wiki page as well. Maybe we can title the page as "Debug in Production" or "Debug Exported App"?

I just updated the Wiki setting so everyone can contribute now, and I think the page should include this:

Dev-Server: electron . --force-devtools
Linux AppImage: ./Manta-1.1.2-x86_64.AppImage --force-devtools
Installed App (.deb): manta --force-devtools

Copy link
Author

@ghost ghost Mar 6, 2018

Choose a reason for hiding this comment

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

Yes, the (1) "--force-devtools" option is really related to development & bug reporting, but (2) "--disable-hardware-acceleration" is a user setting for the app. So we should split the documentation of these two params to 2 different places. For (1) a "Debug Exported App" wiki page would fit. I would leave infos about (2) in the main readme.

I proposed this, cause i thought it's good to have all app start options documented in the same place. we can place all options in the wiki under the development section, but you wont find it there if you search for regular user app options.

Copy link
Author

Choose a reason for hiding this comment

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

I think it would be better if we could add some kind of explanation for why it's here and what it does.

I don't know what to explain. The title of the section and the paramname explain everything there is to explain, or?
Maybe: To give the user the possiblity to easily set some options for the app ...

Copy link
Owner

Choose a reason for hiding this comment

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

I just added a short description to the README. Please have a look and let me know what you think. If it's ok then this PR is ready to be merged.

@hql287
Copy link
Owner

hql287 commented Mar 6, 2018

Installed App (.deb): manta --force-devtools (works in the same way for macOS & win ...)

I just tested it out on my Mac but this didn't work.

open -a /Applications/Manta.app --force-devtools

Received this message instead:

open: unrecognized option `--force-devtools'

Dev-Server: electron . --force-devtools

Also, the devtools don't need to be forced to open in development mode, the conditional isDev would open them automatically.

@ghost
Copy link
Author

ghost commented Mar 6, 2018

I just tested it out on my Mac but this didn't work.

Mac has it's own non standard way, as always :) Try:

open -a /Applications/Manta.app --args --force-devtools

Also, the devtools don't need to be forced to open in development mode, the conditional isDev would open them automatically.

Yes and No. isDev detects the development env, forceDevtools is to open them in production mode. So forceDevtools has nothing to do with the development mode ...

But yeah, following example is misleading somehow:

Dev-Server: electron . --force-devtools

this command with the "--force-devtools" makes no sense, it was just an example how these params work in general.

Dev-Server: electron . --disable-hardware-acceleration

Would be a better example :)

@hql287
Copy link
Owner

hql287 commented Mar 6, 2018

@jens-t Hm, I tried this and it opened the app but the devtools didn't show up.

open -a /Applications/Manta.app --args --force-devtools

@ghost
Copy link
Author

ghost commented Mar 6, 2018

Maybe you should take a look here:
https://superuser.com/questions/180995/passing-command-line-args-to-open-on-mac

the --args option exists since osx version 10.6 , maybe that's the problem?

@hql287
Copy link
Owner

hql287 commented Mar 6, 2018

@jens-t Sorry my bad, I ran this command with the current version of Manta which didn't have the code presented in this PR 😆. Will export a new app and try again.

@ghost
Copy link
Author

ghost commented Mar 6, 2018

Tested with macOS 10.13.2
open -a /Applications/Manta.app --args --force-devtools
works! :)

@ghost
Copy link
Author

ghost commented Mar 6, 2018

Changed the readme, better?

@hql287
Copy link
Owner

hql287 commented Mar 7, 2018

@jens-t: I just added a Wiki page for this. Please double check if I got the Linux commands right and also provide the command for Windows user if you have it.

@ghost
Copy link
Author

ghost commented Mar 7, 2018

I like that page! :) I test it on windows later and add that to the wiki page.

do i have to fork the wiki, or just clone and push to origin?

@ghost
Copy link
Author

ghost commented Mar 7, 2018

I also like your readme changes/additions!

@hql287
Copy link
Owner

hql287 commented Mar 7, 2018

I think you can edit it directly. There should be an "Edit" button next to the green "New Page" button.

@ghost
Copy link
Author

ghost commented Mar 7, 2018

Ah yes, found it, thanks!

@hql287
Copy link
Owner

hql287 commented Mar 7, 2018

I also like your readme changes/additions!

Okay, I'll go ahead and merge this first. Feel free to update that Wiki page whenever you have the time.

@hql287 hql287 merged commit 929db27 into hql287:dev Mar 7, 2018
@ghost
Copy link
Author

ghost commented Mar 7, 2018

Will do!

@ghost
Copy link
Author

ghost commented Mar 7, 2018

Just for your information, no need to change anything:
Just found out, you don't have to use the open command on macOS, you can also type the path to the executable located in the program folder and use it the same way as in linux.

@hql287
Copy link
Owner

hql287 commented Mar 7, 2018

Just found out, you don't have to use the open command on macOS, you can also type the path to the executable located in the program folder and use it the same way as in linux.

Not sure about this, because on my system, typing any path into the terminal would result in the cd command.

Update: Yup, I'm now in the /Application/Manta.app folder 😄

@ghost
Copy link
Author

ghost commented Mar 7, 2018

Read it somewhere, maybe the person used a custom shell. Good to know :)

@ghost ghost deleted the cmdline-flags branch March 7, 2018 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants