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

trust cells with no output #5087

Merged
merged 3 commits into from Feb 12, 2014
Merged

trust cells with no output #5087

merged 3 commits into from Feb 12, 2014

Conversation

minrk
Copy link
Member

@minrk minrk commented Feb 10, 2014

It's outputs we don't trust, so if there are no outputs there's nothing to distrust.

It's outputs we don't trust, so if there are no outputs there's nothing to distrust.
@ivanov
Copy link
Member

ivanov commented Feb 10, 2014

This looks reasonable. In testing this, I think we want to also trust cells with only stream stdout and stderr. Those currently cause a notebook to be untrusted until you rerun them, but no visual indication of the fact that they are untrusted, like the kind that comes with rich output, since the way we display them is safe.

Should we do that here or as a separate PR?

@minrk
Copy link
Member Author

minrk commented Feb 10, 2014

Good idea. This PR now matches more closely the javascript logic - safe (not parsed-safe, but always-safe keys) outputs will not affect the trust result. So you only need to re-run the actual untrusted outputs for the notebook to become signed.

@@ -40,17 +40,7 @@
"cell_type": "code",
"collapsed": false,
"input": [
"next_paragraph = \"\"\"\n",
"Aenean vitae diam consectetur, tempus arcu quis, ultricies urna. Vivamus venenatis sem \n",
"quis orci condimentum, sed feugiat dui porta.\n",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you write this hilarity? Google tells me it translates to "Just remember the main platform of life, the time with the bow person who, in a glass jar. Customizable settings to improve the lot of the average, but the overall performance"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ivanov
Copy link
Member

ivanov commented Feb 12, 2014

ok, tested it, works great, thanks @minrk 👍 merging

ivanov added a commit that referenced this pull request Feb 12, 2014
trust cells with no output
@ivanov ivanov merged commit f7d6d5f into ipython:master Feb 12, 2014
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
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