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

activate.csh handle newlines & don't use basename #1200

Merged
merged 3 commits into from Oct 30, 2018

Conversation

Projects
None yet
2 participants
@gregory-nisbet
Contributor

gregory-nisbet commented Aug 31, 2018

Current activate.csh script is not robust against newlines in the prompt and uses basename unnecessarily.

For example, this .tcshrc file contains a prompt that breaks the script

onintr -

set tcshrc_nl = '\
'

 if ($?prompt) then
    set ellipsis = "..."
    set prompt = "$tcshrc_nl:q""%B[%.03]%b %# "
 endif

onintr

I modified the script to detect the presence of newlines and avoid changing the prompt at all in those cases.

The correct thing to do would probably be to add '[...]' to the beginning of the last line of the prompt, but I don't know of any easy way to accomplish that with csh. I briefly looked into s///-type substitution and using a temporary array variable to split the string into lines, but couldn't get it to work.

tcsh and csh also have the :t modifier for returning the last /-delimited component from a path, e.g.

[~] > set a = /a/b/c/d
[~] > echo $a:t
d

Also, I replaced added :q to every variable expansion (except for the $?varname ones) to guard against characters like newlines.

@gregory-nisbet gregory-nisbet changed the title from activate.csh handle newlines -- don't use basename to activate.csh handle newlines & don't use basename Aug 31, 2018

@gaborbernat gaborbernat force-pushed the pypa:master branch 5 times, most recently from 9b9e4aa to 9d8d241 Oct 22, 2018

@gregory-nisbet

This comment has been minimized.

Contributor

gregory-nisbet commented Oct 26, 2018

@gaborbernat The allow edits box on the sidebar is checked and I believe it was checked originally. I’m not sure why you’re having trouble editing the PR. What are you seeing when you try to edit it?

If you’d prefer, I can make the changes you suggest as a workaround.

@gaborbernat

This comment has been minimized.

Contributor

gaborbernat commented Oct 26, 2018

I get a forbidden when I try to push changes 😃

@gregory-nisbet

This comment has been minimized.

Contributor

gregory-nisbet commented Oct 26, 2018

I'm not sure what's going on.

I unchecked the allow edits and waited for a green check mark to appear. Then I rechecked the allow edits and waited for a green check mark to appear. Can you push now?

If you cannot push, are you able to edit the PR through the website?

If neither thing works, I'll be happy to make changes if you post a patch file as a comment or something similar.

@gregory-nisbet

This comment has been minimized.

Contributor

gregory-nisbet commented Oct 26, 2018

I think I know what's happening. Your banner says Contributor not Maintainer or Owner or equivalent. I don't know whether that's right or not, but I don't see an option to make the "edit rights" for the PR more liberal than "Allow Edits From Maintainers".

@gaborbernat gaborbernat force-pushed the pypa:master branch from 2b3f876 to 9dfcb43 Oct 27, 2018

gregory-nisbet and others added some commits Aug 31, 2018

activate.csh handle newlines -- don't use basename
Current `activate.csh` script is not robust against newlines in the prompt and uses `basename` unnecessarily.

For example, this `.tcshrc` file contains a prompt that breaks the script

```
onintr -

set tcshrc_nl = '\
'

 if ($?prompt) then
    set ellipsis = "..."
    set prompt = "$tcshrc_nl:q""%B[%.03]%b %# "
 endif

onintr
```

I modified the script to detect the presence of newlines and avoid changing the prompt at all in those cases.

The correct thing to do would probably be to add '[...]' to the beginning of the last line of the prompt, but I don't know of any easy way to accomplish that with csh. I briefly looked into `s///`-type substitution and using a temporary array variable to split the string into lines, but couldn't get it to work.

`tcsh` and `csh` also have the `:t` modifier for returning the last `/`-delimited component from a path, e.g.

```
[~] > set a = /a/b/c/d
[~] > echo $a:t
d
```

@gaborbernat gaborbernat force-pushed the gregory-nisbet:master branch from b1b76a3 to 73b05b6 Oct 30, 2018

@gaborbernat

This comment has been minimized.

Contributor

gaborbernat commented Oct 30, 2018

Never mind, seems just had the wrong remote 👍

no longer valid

@gaborbernat gaborbernat merged commit bb0c88f into pypa:master Oct 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment