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 file open support #169

Merged
merged 2 commits into from
Feb 27, 2023
Merged

Added file open support #169

merged 2 commits into from
Feb 27, 2023

Conversation

Wakeful-Cloud
Copy link
Contributor

This pull request adds:

  • The ability to launch RARS (in GUI mode) with a file open as the default tab via passing the file path as an argument (This is necessary for desktop integration since it allows one to simply double-click an assembly file to open it in RARS)
  • A c command line switch to force RARS to launch in CLI mode (In the event anyone is calling RARS with a file path and no other arguments, but don't want to start the GUI, they should use this flag)

@TheThirdOne
Copy link
Owner

You made it a little difficult to review this because you mixed some refactoring in with the core change. I would greatly prefer two commits.

It seems like any program arguments other than file arguments results in no gui. That is pretty hard to confirm unless you make sure every option has gui = false. I would prefer a check like gui = filenameList.length == args.length.

I would be more comfortable with a g option to force a gui. I would expect nearly all scripts to use some arguments, but breaking existing any scripts people are using would leave a bad taste in my mouth. Is there a reason that an argument cannot be included for clicking files?

@Wakeful-Cloud
Copy link
Contributor Author

@TheThirdOne You're absolutely right - I've switched to only enabling the GUI if no arguments are passed (Current behavior) or if the g flag is passed, as well as split the changes into two commits.

@TheThirdOne TheThirdOne merged commit 5868f71 into TheThirdOne:master Feb 27, 2023
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.

None yet

2 participants