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

Instructions on how you envisioned this working? issues with flags.. i.e. --version doesn't work #7

Closed
hanoii opened this issue Sep 6, 2023 · 8 comments · Fixed by #11
Assignees

Comments

@hanoii
Copy link

hanoii commented Sep 6, 2023

I came here from drush-ops/drush-launcher#105, I downloaded it on a docker container I have two codebases.

Ideally, I'd like it to use it as the previous launcher, where I'd call drush anywhere and it will pick up the drupal one. I assume I could rename this launcher to drush and put it somewhere in the path and it should work, haven't tested this though so pointers appreciated.

The one thing that didn't work is
drush-launcher --version, it outputs the error from the launcher. I obviously tried drush-launcher -- --version which worked, but It would be ideal if it would be a default replacement.

Thanks for the contribution.

@dasginganinja
Copy link
Owner

Hi Hanoii I need to take a look and see how this is working in those conditions.

I don't think it's the intended functionality and will take a look into this hopefully this week.

Thanks,
Tom

@dasginganinja
Copy link
Owner

Hi Hanoii, I'm finally getting back around to this.

It appears that the flag module in Go parses options until the first non-option is discovered. It does respect the double dash as the end of the flags, as expected.

I'll do some more work on this piece since obviously this isn't working as expected.

I've finally found some time to look into this. Sorry for the delay.

@dasginganinja
Copy link
Owner

I'll do some digging, but the docs indicate that I can use this to capture the flags I've not set.
flag.CommandLine.ParseErrorsWhitelist = flag.ParseErrorsWhitelist{ UnknownFlags: true, }

This has to be done before flag.parse, which is done Here

From then I can parse the list and add the options back in.

I think I understand the issue at this point. Let me see if I can dig in a bit further.

Thanks, Tommy.

@dasginganinja
Copy link
Owner

I'm approaching a point where I need to replace this on our internal servers and will have some more time to actually look into this. We need the switch options as well.

I assume by end of year I should have this wrapped up and will likely be using it in a production environment.

@tvlooy
Copy link

tvlooy commented Dec 5, 2023

@dasginganinja I wanted to let you know that I wrote a bash alternative based on your approach https://github.com/tvlooy/drush-launcher It seemed less complex / easier to maintain. I don't know what the advantages are of doing this in Go?

@dasginganinja
Copy link
Owner

@tvlooy I had implemented this in Bash at first. I wanted to have a framework for something testable as well. Go seemed like a fantastic framework for that.

I took a look at this code last week that I had proposed in the branch and that didn't work. I'll see if we can get that fixed this week since we finally have time to put some eyes on this.

Thanks,
Tom

@dasginganinja
Copy link
Owner

I think this is ready for review if anybody wants to run go build to test locally.

@dasginganinja dasginganinja linked a pull request Dec 18, 2023 that will close this issue
@dasginganinja
Copy link
Owner

It is now not required to use double dashes -- to separate the arguments. I'll do some testing in some other environments and will probably bring this in / tag 1.0.1 so this can be properly usable.

dasginganinja added a commit that referenced this issue Dec 19, 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 a pull request may close this issue.

3 participants