Add libmpdel-get-state to run handler after state is get

New Pull Request

Conversation 0 Commits 1 Files Changed 1

Fix: #30

I add a help function to wait the value changing to non-nil. To avoid blocking main thread, a timer is used. I have no better ideas to do this.

vonfry added 1 commit 5 months ago

8547fca583

Add libmpdel-get-state to run handler after state is get

vonfry force-pushed fix/play-pause from 8547fca583 to 695f8da40f 5 months ago

DamienCassou approved these changes 5 months ago

Thank you very much. I suggested some changes.

libmpdel.el Outdated

												
					  (let* ((fstate (intern (format "libmpdel-%s" state)))
				
					         timer)
				
					    (unless (libmpdel-connected-p)
				
					      (libmpdel-refresh-status))

DamienCassou commented 5 months ago

I suggest adding an optional callback to libmpdel-refresh-status. This callback would be called when refreshing is finished.

Then you need to pass the lambda below as argument to libmpdel-refresh-status.

If you do that, the code below don't need a timer anymore as it is only executed when the value is already there (you have to replace the unless above with if).

vonfry force-pushed fix/play-pause from 695f8da40f to 4565e120c4 5 months ago

vonfry requested review from DamienCassou 5 months ago

DamienCassou approved these changes 5 months ago

Here are some more suggestions. Thank you for your great work!

libmpdel.el

DamienCassou commented 5 months ago

I suggest passing the variable value as argument to handler. Something like (untested):

(let ((call-handler (lambda ()
                      (funcall handler (funcall fstate)))))
  (if (...)
      (funcall call-handler)
    (libmpdel-refresh-status call-handler)))

vonfry marked this conversation as resolved

libmpdel.el

DamienCassou commented 5 months ago

I suggest only calling libmpdel-send-command once:

(libmpdel-send-command "status"
  (lambda (data)
    (libmpdel--msghandler-status data)
    (when handler (funcall handler))))

vonfry marked this conversation as resolved

vonfry force-pushed fix/play-pause from 4565e120c4 to 8a4adeef79 5 months ago

vonfry force-pushed fix/play-pause from 8a4adeef79 to 27f87b70a1 5 months ago

vonfry force-pushed fix/play-pause from 27f87b70a1 to b0f485e049 5 months ago

vonfry requested review from DamienCassou 5 months ago

vonfry force-pushed fix/play-pause from b0f485e049 to 928857b394 5 months ago

DamienCassou approved these changes 4 months ago

Perfect, thank you very much!

DamienCassou merged commit 0ccf76c860 into master 4 months ago

DamienCassou referenced this issue from a commit 4 months ago

Merge pull request 'Add libmpdel-get-state to run handler after state is get' (#31) from vonfry/libmpdel:fix/play-pause into master

Reviewers

DamienCassou approved these changes 4 months ago

The pull request has been merged as 0ccf76c860.

Sign in to join this conversation.

DamienCassou

No Label

No Milestone

No Assignees

2 Participants

Notifications

Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

 failed due to the state is nil."
   (let ((fstate (intern (format "libmpdel-%s" state))))
     (if (and (funcall fstate) (libmpdel-connected-p))
         (funcall handler)

      (pause "pause 0")
      (stop "play"))))
   (libmpdel-get-state 'play-state
     (lambda ()

 If HANDLER is not nil, it will be called after refreshing."
   (if handler
       (libmpdel-send-command "status"

#31 Add libmpdel-get-state to run handler after state is get

Reviewers