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

Disable cache #48

Merged
merged 7 commits into from
May 12, 2017
Merged

Disable cache #48

merged 7 commits into from
May 12, 2017

Conversation

TatriX
Copy link
Contributor

@TatriX TatriX commented May 12, 2017

Fix #34

@TatriX
Copy link
Contributor Author

TatriX commented May 12, 2017

Usage:
M-x indium-webkit-set-cache-disabled to disable cache.
C-u M-x indium-webkit-set-cache-disabled to enable it back.
Also interactive call will save your choice and it will be applied on next indium launch for a current session.
You can make it permament by setting

(setq indium-webkit-cache-disabled t)

indium-webkit.el Outdated
@@ -40,6 +40,7 @@
(require 'indium-debugger)
(require 'indium-workspace)

(defvar indium-webkit-cache-disabled nil)
Copy link
Owner

Choose a reason for hiding this comment

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

Could you add a docstring for it?

indium-webkit.el Outdated
"Disable network cache for each request. If prefix ARG is used enable it."
(interactive "p")
(setq indium-webkit-cache-disabled (= arg 1))
(indium-webkit--set-cache-disabled indium-webkit-cache-disabled))
Copy link
Owner

Choose a reason for hiding this comment

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

Using a prefix argument feels a bit weird IMO.

What about indium-webkit-enable-cache and indium-webkit-disable-cache instead?

(indium-webkit--send-request '((method . "Network.enable"))
(lambda (res)
(when indium-webkit-cache-disabled
(indium-webkit--set-cache-disabled t)))))
Copy link
Owner

Choose a reason for hiding this comment

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

Could you add documentation for the new commands and this behavior?

Copy link
Owner

@NicolasPetton NicolasPetton left a comment

Choose a reason for hiding this comment

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

Thanks for adding documentation! I left a few remarks

indium-webkit.el Outdated
(setq indium-webkit-cache-disabled (= arg 1))
(indium-webkit--set-cache-disabled indium-webkit-cache-disabled))
(defun indium-webkit-enable-cache ()
"Enabled network cache for each request. And save it for future sessions."
Copy link
Owner

Choose a reason for hiding this comment

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

I'm being picky here, but I'd like the second sentence to start on a separate line, and not starting with "And".
Also, what does "future sessions" mean? It's not persistent if the user restarts Emacs for instance.

M-x indium-webkit-disable-cache
M-x indium-webkit-enable-cache

Both commands save your choice which will be applied on next indium launch for a current session.
Copy link
Owner

Choose a reason for hiding this comment

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

Both commands save your choice which will be used for future Indium connections for the current Emacs session?

@NicolasPetton
Copy link
Owner

indium.info should not be part of the PR. Other than that, it's good, thanks!

This PR seems to be on top of #46, so I'll wait to it to be merged before merging this one.

indium-webkit.el Outdated
@@ -40,6 +40,8 @@
(require 'indium-debugger)
(require 'indium-workspace)

(defvar indium-webkit-cache-disabled nil
"Network cache disabled state. If t disable cache when Indium starts.")
Copy link
Owner

Choose a reason for hiding this comment

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

it should say if non-nil.

M-x indium-webkit-disable-cache
M-x indium-webkit-enable-cache

Both commands save your choice which will be used for future Indium connections for the current Emacs session
Copy link
Owner

Choose a reason for hiding this comment

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

Missing punctuation dot.

Copy link
Owner

@NicolasPetton NicolasPetton left a comment

Choose a reason for hiding this comment

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

Awesome, thanks a lot!

@NicolasPetton NicolasPetton merged commit 48ab791 into NicolasPetton:master May 12, 2017
TatriX added a commit to TatriX/Indium that referenced this pull request May 13, 2017
* Add indium-webkit-{enable|disable}-cache (see NicolasPetton#34).
* Add a new documentation page about the network.
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