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

Use lisp-style parens and indentation #3

Open
thomasf opened this issue Apr 20, 2015 · 9 comments
Open

Use lisp-style parens and indentation #3

thomasf opened this issue Apr 20, 2015 · 9 comments

Comments

@thomasf
Copy link
Contributor

thomasf commented Apr 20, 2015

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

@thomasf
Copy link
Contributor Author

thomasf commented Apr 20, 2015

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

@thomasf
Copy link
Contributor Author

thomasf commented Apr 20, 2015

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
Copy link
Contributor Author

thomasf commented Apr 20, 2015

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

@thomasf thomasf changed the title Remove (message on startup. Some bugs and stuff Apr 20, 2015
@thomasf
Copy link
Contributor Author

thomasf commented Apr 20, 2015

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.

@alanhamlett
Copy link
Member

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 7f2aafe.

Thanks!

@thomasf
Copy link
Contributor Author

thomasf commented Apr 21, 2015

ah, I see. thats a good one then.

@thomasf
Copy link
Contributor Author

thomasf commented Apr 21, 2015

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
Copy link
Contributor Author

thomasf commented Apr 21, 2015

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.

@alanhamlett
Copy link
Member

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 alanhamlett reopened this Apr 21, 2015
@alanhamlett alanhamlett changed the title Some bugs and stuff Use lisp-style parens and indentation Apr 11, 2017
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

No branches or pull requests

2 participants