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

Make : invalid in filenames in the Notebook JS code. #1815

Merged
merged 1 commit into from Jun 6, 2012

Conversation

ellisonbg
Copy link
Member

This only prevents : in the filenames on the JavaScript side of things. Handling this on the server side will be a separate issue that is related to other open issue. I will update those to reflect this. Covers #1781.

@ellisonbg
Copy link
Member Author

This addresses #1781.

@Carreau
Copy link
Member

Carreau commented Jun 1, 2012

Wouldn't it be safer to make a notebook whitelist caracters (a-zA-Z0-9_.)?
Right now it seem to work with lots of caracters, but won't we ran into more issues when filename will start having éêßîü inside, not at webserver level, but filesystem level ? ( who said windows and spaces ?)

@minrk
Copy link
Member

minrk commented Jun 1, 2012

Unicode should be fine, and we definitely shouldn't force ascii. What should happen is failures to save with a particular name should be well-represented to the user, and not result in data-loss (certainly a failed rename should not result in removal of the original).

@Carreau
Copy link
Member

Carreau commented Jun 2, 2012

I was thinking more of problem that can arrise with shell, when using space,$ and some other caractères.
For example a notebook name starting with #, it will be seen as a bash commentary in script.
But I guess this is more a discusion of filename vs notebook title, but as right now the two are the same we could allow any kind of caracters.

@minrk
Copy link
Member

minrk commented Jun 2, 2012

I was thinking more of problem that can arrise with shell, when using space,$ and some other caractères.
For example a notebook name starting with #, it will be seen as a bash commentary in script.
But I guess this is more a discusion of filename vs notebook title, but as right now the two are the same we could allow any kind of caracters.

But the notebook (and Python in general) has no problem at all with such things. If users are doing lots of bash scripting with their notebook files, they might choose space-less ASCII names for convenience. But there is no reason we should enforce something like that.


Reply to this email directly or view it on GitHub:
#1815 (comment)

minrk added a commit that referenced this pull request Jun 6, 2012
Make : invalid in filenames in the Notebook JS code.

This only prevents : in the filenames on the JavaScript side of things. Handling this on the server side will be a separate issue that is related to other open issue. I will update those to reflect this. 

closes #1781
@minrk minrk merged commit 66a567b into ipython:master Jun 6, 2012
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Make : invalid in filenames in the Notebook JS code.

This only prevents : in the filenames on the JavaScript side of things. Handling this on the server side will be a separate issue that is related to other open issue. I will update those to reflect this. 

closes ipython#1781
smoofra pushed a commit to smoofra/emacs-ipython-notebook that referenced this pull request Mar 21, 2015
The JS client code was changed in this PR:
ipython/ipython#1815
(commit: ellisonbg/ipython@e264b9b)
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

3 participants