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

Loading rc in non-interactive login shell #3422

Merged
merged 15 commits into from
Mar 1, 2020
Merged

Conversation

t0fik
Copy link
Contributor

@t0fik t0fik commented Jan 15, 2020

Not loading rc files in exec mode was causing problems with setting up environment starting X session.

Changes:

@gforsyth
Copy link
Collaborator

Hey @t0fik -- thanks for putting this in! Can you rebase/merge master in? There are some spurious CI failures that we can eliminate that way. Thanks!

@t0fik
Copy link
Contributor Author

t0fik commented Jan 30, 2020

@gforsyth master merged.

@anki-code
Copy link
Member

@t0fik thanks for your work! Is this PR could fix #3443 ? Or I'm not clearly understand (could you give some cases).

@AstraLuma
Copy link
Member

@anki-code when xonsh starts, it can be:

  • Interactive or non-interactive
  • Login or non-login

The old behavior only loaded RC files for interactive shells. This PR additionally loads RC files in the non-interactive+login case.

@t0fik
Copy link
Contributor Author

t0fik commented Feb 14, 2020

@anki-code @astronouth7303 Exactly. The behaviour of Bash is the same.
Ex.
xonsh -c 'some command' - does not load rc files
xonsh -l -c 'some command' - loads environment from rc

@anki-code
Copy link
Member

anki-code commented Feb 14, 2020

xonsh -l -c 'some command' - loads environment from rc

@t0fik great! Thanks! Am I right that

xonsh -l -rc rcfile1 rcfile2 -c 'print(var_from_rc)'

will works also?

@t0fik
Copy link
Contributor Author

t0fik commented Feb 14, 2020

@anki-code Yes, it will work that way.
I've only disabled setting rc to None when -l switch was used. Rest of the process of loading rc files is unchanged.

@scopatz
Copy link
Member

scopatz commented Feb 28, 2020

Going to close/reopen to trigger CI

@scopatz scopatz closed this Feb 28, 2020
@scopatz scopatz reopened this Feb 28, 2020
@scopatz
Copy link
Member

scopatz commented Feb 28, 2020

Hi @t0fik - this seems to have some problems on Windows and needs black to be run on it

@t0fik
Copy link
Contributor Author

t0fik commented Feb 28, 2020

@scopatz I fixed tests on Windows and ran black. Other problems seems to be not related with this PR.
I've also merged master.

@scopatz
Copy link
Member

scopatz commented Feb 29, 2020

So I have restarted CI a few times, and I think this is related to the test failures. Can you take a closer look please?

@t0fik
Copy link
Contributor Author

t0fik commented Mar 1, 2020

@scopatz My code is all green now.

@gforsyth
Copy link
Collaborator

gforsyth commented Mar 1, 2020

Thanks for pushing on this @t0fik !! This looks great.

@gforsyth gforsyth merged commit b37675a into xonsh:master Mar 1, 2020
@gforsyth
Copy link
Collaborator

gforsyth commented Mar 1, 2020

Also, I see this is your first contribution to xonsh. Thank you for the contribution and welcome!!

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

5 participants