-
Notifications
You must be signed in to change notification settings - Fork 39
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
optimizations and fixes #142
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just slight feedback needed on some of the changes.
@@ -384,6 +384,8 @@ end | |||
function get_file() | |||
local path = mp.get_property('path') | |||
if not path then return end | |||
-- ignoring protocols that do not support playback recovery |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this only to ignore these protocols from saving to history?
Or these protocols cause a problem if it was added to history?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to the limitations of mpv these protocols can't resume playback from a record, and there's little benefit to saving these invalid entries other than polluting the history list.
Ref: https://mpv.io/manual/master/#protocols
@@ -371,8 +373,6 @@ function parse_clipboard(text) | |||
if string.match(c, '(.*)'..c_pre_attribute) then | |||
c_clip_file = string.match(c_protocols, '(.*)'..c_pre_attribute) | |||
c_clip_time = tonumber(string.match(c_protocols, c_pre_attribute..'(%d*%.?%d*)')) | |||
elseif string.match(c, '^\"(.*)\"$') then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for the removal of this condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this has been processed in the make_raw
function, no additional processing is required.
@@ -385,8 +385,6 @@ function parse_clipboard(text) | |||
if string.match(c, '(.*)'..c_pre_attribute) then | |||
c_clip_file = string.match(c, '(.*)'..c_pre_attribute) | |||
c_clip_time = tonumber(string.match(c, c_pre_attribute..'(%d*%.?%d*)')) | |||
elseif string.match(c, '^\"(.*)\"$') then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for the removal of this condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same reason
There seems to be a bug with github and my reply turned into self-censorship... |
Any progress? |
Can I have the clarifications in the review section for the questions I asked? |
Of course. |
Are there still some github bugs ? because I couldn't see your feedback on Eisa01's review.. |
No description provided.