wakatime-mode icon indicating copy to clipboard operation
wakatime-mode copied to clipboard

Use lisp-style parens and indentation

Open thomasf opened this issue 9 years ago • 9 comments

It's just annoying. Use it for errors, nothing else.

thomasf avatar Apr 20 '15 20:04 thomasf

Also, the message "Initializing WakaTime v1.0.0" happens very often,

thomasf avatar Apr 20 '15 22:04 thomasf

I think you might be misunderstaning the if form because it is work like this:

(if cond
    then
  else...)

The correct way to have more than one expression in the then part of an if expression is to wrap it inside a progn:

(defun wakatime-init ()
  (if (not wakatime-initialized)
      (progn
        (message "Initializing WakaTime v%s" wakatime-version)
        (if (or (not wakatime-api-key) (string= "" wakatime-api-key))
            (wakatime-prompt-api-key))
        ...
        (setq wakatime-initialized t))))

Since you don't have any else expressions you can use the when macro which adds the progn for you:

(defun wakatime-init ()
  (when (not wakatime-initialized)
    (message "Initializing WakaTime v%s" wakatime-version)
    (if (or (not wakatime-api-key) (string= "" wakatime-api-key))
        (wakatime-prompt-api-key))
    ...
    (setq wakatime-initialized t)))

And finally, there is also an inversed version of the when macro called unless which can remove the need for not

(defun wakatime-init ()
  (unless wakatime-initialized
    (message "Initializing WakaTime v%s" wakatime-version)
    (if (or (not wakatime-api-key) (string= "" wakatime-api-key))
        (wakatime-prompt-api-key))
    ...
    (setq wakatime-initialized t)))

thomasf avatar Apr 20 '15 23:04 thomasf

Also, many emacs users get any changes at once when they are committed to master because melpa auto packages them http://melpa.org/

thomasf avatar Apr 20 '15 23:04 thomasf

You should probably not try to protect from recursive miibuffer calls..

The default value of enable-recurive-minibuffers in emacs is nil, if a user has it enabled s/he better know how to handle it.

thomasf avatar Apr 20 '15 23:04 thomasf

You should probably not try to protect from recursive miibuffer calls

With enable-recurive-minibuffers set to nil reading input from a minibuffer causes a recursive loop when WakaTime tries to initialize itself inside the new minibuffer.

I've removed the initializing message. I thought it went to the *messages* buffer without the user seeing it.

These changes are fixed with 7f2aafefb5909767ecee2b8a750d516e166d1117.

Thanks!

alanhamlett avatar Apr 21 '15 00:04 alanhamlett

ah, I see. thats a good one then.

thomasf avatar Apr 21 '15 00:04 thomasf

The shell command which wakatime does the same things as the elisp function (executable-find "wakatime") . There is also a file-executable-p function that does not look in the exec path.

The wakatime-noprompt probably not needed... You can also check if the current buffer is the minibuffer with a call to (minibufferp) instead.

thomasf avatar Apr 21 '15 00:04 thomasf

The wakatime-call function is really really hard to read in it's current state since it both indented the wrong way and uses line breaks in unconventional ways. But that's not a real issue so don't focus on that..

Given the current layout the indentation should look like

(defun wakatime-call (command)
  "Call WakaTime COMMAND."
  (let
      (
       (process
        (start-process
         "Shell"
         (generate-new-buffer " *WakaTime messages*")
         shell-file-name
         shell-command-switch
         command
         )
        )
       )

    (set-process-sentinel process
                          (lambda (process signal)
                            (when (memq (process-status process) '(exit signal))
                              (let ((exit-status (process-exit-status process)))
                                (when (and (not (= 0 exit-status)) (not (= 102 exit-status)))
                                  (error "WakaTime Error (%s)" exit-status)
                                  )
                                )
                              )
                            )
                          )

    (set-process-query-on-exit-flag process nil)
    )
  )

The old formatting was more idiomatic in every way. It just takes some time to get used to the lispy style.

thomasf avatar Apr 21 '15 00:04 thomasf

Ok, I'll use lisp-style parens. Leaving this open as a reminder.

Thanks for the executable find function, that's much better than which.

alanhamlett avatar Apr 21 '15 06:04 alanhamlett