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

Support rez-env -c <alias> or -- <alias> (Windows CMD shell) #948

Conversation

davidlatwe
Copy link
Contributor

@davidlatwe davidlatwe commented Sep 2, 2020

Problem

As #708 stated, rez-env pkg -c alias-cmd or rez-env pkg -- alias-cmd currently not supported (on Windows).

Solution

Since the Alias object will be saved in ActionManager if any alias setup been found in package's commands by RexExecutor, look into that list and matching them with input command, swap it back to actual command if matched, would be the simplest solution.

@davidlatwe davidlatwe changed the title Support alias in rez-env -c or -- c Support alias in rez-env -c <alias> or -- <alias> (on Windows) Sep 2, 2020
@davidlatwe davidlatwe changed the title Support alias in rez-env -c <alias> or -- <alias> (on Windows) Support rez-env -c <alias> or -- <alias> (on Windows) Sep 2, 2020
@instinct-vfx
Copy link
Contributor

I think this would rather belong into the cmd shell plugin instead of resolved_context. It is a shell specific issue, not platform specific. (Powershell handles it fine).

@davidlatwe
Copy link
Contributor Author

davidlatwe commented Sep 3, 2020

That's weird, just tested on current master branch with my powershell and it didn't work :O

Ah, nevermind. Just found that I am still spawning cmd shell in my powershell. :/

e.arg[0] may be an int in some error type
@davidlatwe
Copy link
Contributor Author

Okay, I have changed my implementation into CMD specific. ☺️

@davidlatwe davidlatwe changed the title Support rez-env -c <alias> or -- <alias> (on Windows) Support rez-env -c <alias> or -- <alias> (Windows CMD shell) Sep 3, 2020
@davidlatwe
Copy link
Contributor Author

Just checking in, is this a valid/solid solution to you guys ? :)

@davidlatwe
Copy link
Contributor Author

Just improved the implementation, the command that has alias + arguments can be matched and expand now.

Also, one more simple test case is added.

@nerdvegas nerdvegas merged commit a6a2364 into AcademySoftwareFoundation:master Oct 5, 2020
@davidlatwe davidlatwe deleted the issue_708-swap-alias-command branch November 17, 2020 06:09
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.

3 participants