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 setting the notebook dirty flag an event #3371

Merged
merged 5 commits into from Jun 2, 2013

Conversation

minrk
Copy link
Member

@minrk minrk commented May 28, 2013

also adds a more liberal setting of the dirty flag when the user is typing, to avoid the current state of things where editing a code cell has no effect on the dirty flag unless it is executed.

minrk added 2 commits May 28, 2013 12:43
simple, liberal check for whether a keypress is probably typing or not.
COMMAND : 91,
LEFT_SUPER : 91,
RIGHT_SUPER : 92,
COMMAND : 93,
Copy link
Member

Choose a reason for hiding this comment

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

COMMAND from 91to 93 ?
on mine COMMAND (ex Apple key right?) is 91

Copy link
Member

Choose a reason for hiding this comment

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

Correction, LEFT_COMMAND is 91 RIGHT_COMMAND is 93 (mbp 13" mid 2012)

http://www.cambiaresearch.com/articles/15/javascript-char-codes-key-codes

Copy link
Member Author

Choose a reason for hiding this comment

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

I used that reference as well - my keyboard (Kinesis Advantage) sends 93 for command, and the OS interprets that as command as well.

@Carreau
Copy link
Member

Carreau commented May 28, 2013

Why not bind to CM change event ?

@minrk
Copy link
Member Author

minrk commented May 28, 2013

Why not bind to CM change event ?

I will do that instead, and remove the silly utility function

@Carreau
Copy link
Member

Carreau commented May 29, 2013

Looks fine to me.
Just maybe s/set_dirty(true)/set_dirty()/g , and do you want a set_clean alias or equivalent ?

instead of last-saved timestamp
@@ -148,9 +149,12 @@ var IPython = (function (IPython) {
);
}

SaveWidget.prototype.set_last_saved = function () {
var d = new Date();
this.set_save_status('(autosaved: '+d.format('mmm dd HH:MM') + ')');
Copy link
Member

Choose a reason for hiding this comment

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

Having time was great ! May I suggest somethig like http://momentjs.com/ ? Can be done as another PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we specifically want to get away from this notion of autosave times. Either the document has been autosaved, or some small amount of recent changes have not. I don't think we want to be presenting two timestamps for every file.

Copy link
Member Author

Choose a reason for hiding this comment

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

But if we decide to keep the timestamp, I can always just add the "unsaved changes" message, but the save widget's text is already too long, and needs to be shorter.

Copy link
Member

Choose a reason for hiding this comment

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

It's fine I can do it with my custom.js if I need to. But it is true that now you have the 2 status.
I just have the bad habit of soemetime having the same notebook opened on multiple window... so I use this to know which one is the "last" saved.

Carreau added a commit that referenced this pull request Jun 2, 2013
make setting the notebook dirty flag an event
@Carreau Carreau merged commit 7d96d7a into ipython:master Jun 2, 2013
@minrk minrk deleted the dirty-event branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
make setting the notebook dirty flag an event
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

2 participants