FAQ
This is great. Thank you very much for doing this. It will be great to
have a mode that integrates better with standard Emacs mechanisms, and
the other features are great, too.

Many of my comments below are stylistic, but I did have some trouble
getting some of the features to work. I may have a configuration that
is non-standard in some way, but I'm not sure. I started with a fresh
instance of GNU Emacs 24.2.1 with no other customizations, then
installed the latest version from
<https://github.com/dominikh/go-mode.el>.


https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el
File misc/emacs/go-mode.el (left):

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#oldcode885
misc/emacs/go-mode.el:885: (defun godoc (query)
I couldn't get this to work. It failed silently. Maybe there's some
configuration I need to do. Consider adding more documentation about
the assumptions at the top of the file.

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el
File misc/emacs/go-mode.el (right):

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode17
misc/emacs/go-mode.el:17: (defconst go-func-meth-regexp (concat
"\\<func\\>\\s *\\(?:(\\s *" go-identifier-regexp "\\s +" go-type-regexp
"\\s *)\\s *\\)?\\(" go-identifier-regexp "\\)("))
It would be nice to make `beginning-of-defun', etc. work with other
top-level constructs, e.g. <type>.

Also, it's okay to have long lines, but sometimes it helps to wrap them
to reveal their structure through indentation. This line is an
example. See `go-mode-keywords' below for a nice example.

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode73
misc/emacs/go-mode.el:73: )
Avoid using closing parentheses on lines by themselves in Lisp.
Instead, move it up to the end of the previous line (before the comment,
of course).

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode131
misc/emacs/go-mode.el:131: (defun go--backward-irrelevant (&optional
stop-at-string)
It would be great to have short documentation strings for non-trivial
functions like this.

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode165
misc/emacs/go-mode.el:165: (setq group "^[{("))
It would be better not to use assignment here and above. Instead, put
the `if' inside the `let'.

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode264
misc/emacs/go-mode.el:264: proper parsing of the buffer content to allow
features such as
"proper" => "and proper"

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode268
misc/emacs/go-mode.el:268: Additionally to these core features, it
offers various features to
"Additionally" => "In addition"

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode291
misc/emacs/go-mode.el:291: recommended to look at goflymake
"to look" => "that you look"

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode344
misc/emacs/go-mode.el:344: (defun gofmt ()
When I run this, it works, but ends with the message "Hunk applied." It
would be better to say something more helpful, or nothing at all.

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode531
misc/emacs/go-mode.el:531: )
Move these closing parentheses up.

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode562
misc/emacs/go-mode.el:562: (defun go-download-play (url)
`go-download-play' fails unless `url-request-data',
`url-request-extra-headers', and `url-request-method' are set (e.g. to
nil).

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode591
misc/emacs/go-mode.el:591: (defun go-import-add (arg import)
`go-import-add' fails with this unhelpful error message unless GOPATH is
set to a directory that contains "pkg/":

(file-error "Opening directory" "no such file or directory" "/pkg/")

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode650
misc/emacs/go-mode.el:650: ))
To match standard Lisp style, please move these closing parentheses up
to the line with `mapconcat'.

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode654
misc/emacs/go-mode.el:654: (go-root-and-paths))) 'string<))
Please break this line after the third closing parenthesis. It's
confusing to have the comparator at an indentation level different than
its peer argument.

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode659
misc/emacs/go-mode.el:659: (setq cmd "go test -c")
It would be better not to use assignment here and below. Instead, put
the `if' inside the `let'.

https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode675
misc/emacs/go-mode.el:675: (defun go-remove-unused-imports (arg)
`go-unused-imports-lines' fails silently if <go build> or <go test>
fails.

https://codereview.appspot.com/7314113/

--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Proppy at Feb 21, 2013 at 12:55 pm
    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode-load.el
    File misc/emacs/go-mode-load.el (left):

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode-load.el#oldcode7
    misc/emacs/go-mode-load.el:7: ;; (require 'go-mode-load)
    would be nice to leave the instructions

    https://codereview.appspot.com/7314113/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Dominik Honnef at Feb 21, 2013 at 4:57 pm
    https://codereview.appspot.com/7314113/diff/14001/misc/emacs/go-mode-load.el
    File misc/emacs/go-mode-load.el (right):

    https://codereview.appspot.com/7314113/diff/14001/misc/emacs/go-mode-load.el#newcode1
    misc/emacs/go-mode-load.el:1: ;;; go-mode-load.el --- Major mode for the
    Go programming language
    On 2013/02/20 06:03:40, cw wrote:
    load or loads here?
    Done.

    https://codereview.appspot.com/7314113/diff/14001/misc/emacs/go-mode-load.el#newcode74
    misc/emacs/go-mode-load.el:74:
    On 2013/02/20 06:03:40, cw wrote:
    again, load vs loads
    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (left):

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#oldcode885
    misc/emacs/go-mode.el:885: (defun godoc (query)
    On 2013/02/21 00:29:29, arthur wrote:
    I couldn't get this to work. It failed silently. Maybe there's some
    configuration I need to do. Consider adding more documentation about the
    assumptions at the top of the file.
    Pretty much the only assumption is that it can run godoc, so PATH has to
    be set correctly. That's about it.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (right):

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode17
    misc/emacs/go-mode.el:17: (defconst go-func-meth-regexp (concat
    "\\<func\\>\\s *\\(?:(\\s *" go-identifier-regexp "\\s +" go-type-regexp
    "\\s *)\\s *\\)?\\(" go-identifier-regexp "\\)("))
    On 2013/02/21 00:29:29, arthur wrote:
    It would be nice to make `beginning-of-defun', etc. work with other top-level
    constructs, e.g. <type>.
    I did consider this, but first checked with other modes and they only
    worked on functions as well, or only sporadically on other structures.

    Either way, if at all, I'd rather do this in a separate CL to avoid
    introducing new changes in behavior that have to be reviewed.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode73
    misc/emacs/go-mode.el:73: )
    On 2013/02/21 00:29:29, arthur wrote:
    Avoid using closing parentheses on lines by themselves in Lisp.
    Instead, move
    it up to the end of the previous line (before the comment, of course).
    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode131
    misc/emacs/go-mode.el:131: (defun go--backward-irrelevant (&optional
    stop-at-string)
    On 2013/02/21 00:29:29, arthur wrote:
    It would be great to have short documentation strings for non-trivial functions
    like this.
    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode165
    misc/emacs/go-mode.el:165: (setq group "^[{("))
    On 2013/02/21 00:29:29, arthur wrote:
    It would be better not to use assignment here and above. Instead, put the `if'
    inside the `let'.
    I wasn't sure having a case within an if within a function call was such
    a good idea, style-wise.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode264
    misc/emacs/go-mode.el:264: proper parsing of the buffer content to allow
    features such as
    On 2013/02/21 00:29:29, arthur wrote:
    "proper" => "and proper"
    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode268
    misc/emacs/go-mode.el:268: Additionally to these core features, it
    offers various features to
    On 2013/02/21 00:29:29, arthur wrote:
    "Additionally" => "In addition"
    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode291
    misc/emacs/go-mode.el:291: recommended to look at goflymake
    On 2013/02/21 00:29:29, arthur wrote:
    "to look" => "that you look"
    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode344
    misc/emacs/go-mode.el:344: (defun gofmt ()
    On 2013/02/21 00:29:29, arthur wrote:
    When I run this, it works, but ends with the message "Hunk applied." It would
    be better to say something more helpful, or nothing at all.
    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode531
    misc/emacs/go-mode.el:531: )
    On 2013/02/21 00:29:29, arthur wrote:
    Move these closing parentheses up.
    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode562
    misc/emacs/go-mode.el:562: (defun go-download-play (url)
    On 2013/02/21 00:29:29, arthur wrote:
    `go-download-play' fails unless `url-request-data',
    `url-request-extra-headers',
    and `url-request-method' are set (e.g. to nil).
    Over here (main Emacs and Emacs with no site lisp/configuration loaded),
    it worked without, too. Is some other package (wrongly) setting these?
    I've seen other packages, including ELPA, fail before because of issues
    regarding the url-* variables.

    Nevertheless, I'm setting them to proper values now.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode591
    misc/emacs/go-mode.el:591: (defun go-import-add (arg import)
    On 2013/02/21 00:29:29, arthur wrote:
    `go-import-add' fails with this unhelpful error message unless GOPATH is set to
    a directory that contains "pkg/":
    (file-error "Opening directory" "no such file or directory" "/pkg/")
    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode650
    misc/emacs/go-mode.el:650: ))
    On 2013/02/21 00:29:29, arthur wrote:
    To match standard Lisp style, please move these closing parentheses up to the
    line with `mapconcat'.
    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode654
    misc/emacs/go-mode.el:654: (go-root-and-paths))) 'string<))
    On 2013/02/21 00:29:29, arthur wrote:
    Please break this line after the third closing parenthesis. It's
    confusing to
    have the comparator at an indentation level different than its peer
    argument.

    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode659
    misc/emacs/go-mode.el:659: (setq cmd "go test -c")
    On 2013/02/21 00:29:29, arthur wrote:
    It would be better not to use assignment here and below. Instead, put the `if'
    inside the `let'.
    Done.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode675
    misc/emacs/go-mode.el:675: (defun go-remove-unused-imports (arg)
    On 2013/02/21 00:29:29, arthur wrote:
    `go-unused-imports-lines' fails silently if <go build> or <go test>
    fails.

    Technically, go build also fails when its doing its job of telling us
    about unused imports. So how do we tell that apart from real failure?

    https://codereview.appspot.com/7314113/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Arthur at Feb 21, 2013 at 6:08 pm
    Thanks for making these changes.


    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (left):

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#oldcode885
    misc/emacs/go-mode.el:885: (defun godoc (query)
    I've confirmed that the command is running, and that its exit value
    indicates success.

    Oh, my mistake. I'm a novice Go programmer, and I misunderstood what
    <godoc> was supposed to do. I was trying it on things like "make" and
    "map", for which it returns no results. When I give it a package name,
    it works.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (right):

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode17
    misc/emacs/go-mode.el:17: (defconst go-func-meth-regexp (concat
    "\\<func\\>\\s *\\(?:(\\s *" go-identifier-regexp "\\s +" go-type-regexp
    "\\s *)\\s *\\)?\\(" go-identifier-regexp "\\)("))
    That's fine. I can always customize this by changing the regular
    expression.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode165
    misc/emacs/go-mode.el:165: (setq group "^[{("))
    In general, functional code is better than side-effecting code,
    especially in simple cases like this.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode562
    misc/emacs/go-mode.el:562: (defun go-download-play (url)
    Strange. I'm running without any customizations or packages loaded
    other than "go-mode.el". In any case, thanks for fixing this.

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#newcode675
    misc/emacs/go-mode.el:675: (defun go-remove-unused-imports (arg)
    I see. That's a good question. It would be nice if <go build> offered
    you a way of calling it just for this purpose so that you could
    distinguish between the two cases. Oh, well. Maybe someone on the Go
    team has a suggestion.

    https://codereview.appspot.com/7314113/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Dominik Honnef at Feb 21, 2013 at 6:21 pm
    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (left):

    https://codereview.appspot.com/7314113/diff/32001/misc/emacs/go-mode.el#oldcode885
    misc/emacs/go-mode.el:885: (defun godoc (query)
    On 2013/02/21 18:08:03, arthur wrote:
    I've confirmed that the command is running, and that its exit value indicates
    success.
    Oh, my mistake. I'm a novice Go programmer, and I misunderstood what <godoc>
    was supposed to do. I was trying it on things like "make" and "map", for which
    it returns no results. When I give it a package name, it works.
    Hm, when I give it a name that doesn't exist (such as "make"), it prints
    a godoc error in the minibuffer; it doesn't fail silently.

    https://codereview.appspot.com/7314113/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Adonovan at Feb 21, 2013 at 6:45 pm
    Dominik, thanks for taking this on. At one point I was tempted to
    switch go-mode.el to use syntax-ppss but I suspected that it was Atlas's
    trick on Hercules and I would be left maintaining this code forever. So
    I'm glad to see you have stepped up. :)

    Your rewrite is quite complex so I'm not sure it's worth the effort to
    understand all the logic; my only code comments are trivial. But I have
    been playing with it and it seems both more accurate and noticeably
    faster than the previous indentation algorithm. It's also shorter.
    Nice work!


    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (left):

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#oldcode111
    misc/emacs/go-mode.el:111: (define-key m "}"
    #'go-mode-insert-and-indent)
    Though #'foo is functionally equivalent to 'foo in Emacs Lisp if foo is
    a function, I would still recommend the traditional #' as it both serves
    as documentation and enables compiler optimisations (since it indicates
    that the name of the symbol isn't significant).

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (right):

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#newcode120
    misc/emacs/go-mode.el:120: `(car (syntax-ppss)))
    Glad to see you're using ppss; I never understood why the old mode
    avoided just about the most optimised C function in Emacs.

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#newcode134
    misc/emacs/go-mode.el:134: (defun go--backward-irrelevant (&optional
    stop-at-string)
    What's the significance of two dashes in go--foo? (Private?)

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#newcode654
    misc/emacs/go-mode.el:654: (defun go-root-and-paths ()
    I recommend adding docstrings even for short internal functions.

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#newcode679
    misc/emacs/go-mode.el:679: ;; FIXME Technically, -o /dev/null fails in
    quite some cases (on
    Prefer "TODO($USER): fix: ..." to "FIXME: ...".

    https://codereview.appspot.com/7314113/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Dominik Honnef at Feb 21, 2013 at 7:08 pm
    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (left):

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#oldcode111
    misc/emacs/go-mode.el:111: (define-key m "}"
    #'go-mode-insert-and-indent)
    On 2013/02/21 18:45:49, adonovan wrote:
    Though #'foo is functionally equivalent to 'foo in Emacs Lisp if foo is a
    function, I would still recommend the traditional #' as it both serves as
    documentation and enables compiler optimisations (since it indicates that the
    name of the symbol isn't significant).
    I actually wasn't too sure about the meaning of #'. Last time I
    researched it, it was said that # causes the compiler to inline
    functions, which in my opinion didn't make much sense for define-key.
    I'd really love some documentation on #'

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (right):

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#newcode120
    misc/emacs/go-mode.el:120: `(car (syntax-ppss)))
    On 2013/02/21 18:45:49, adonovan wrote:
    Glad to see you're using ppss; I never understood why the old mode
    avoided just
    about the most optimised C function in Emacs.
    Heh, two reasons, I'll start from the back: They ditched the syntax
    table completely to properly handle raw strings, which have no escape
    sequences. The syntax table can't handle that. My solution is limited to
    XEmacs and GNU Emacs 24.x and degrades gracefully on older versions.

    The other reason is: The authors of the old go-mode thought, and I
    quote, that emacs has no built-in way to tell you the current nesting in
    parentheses. Or in other words, they probably didn't know syntax-ppss :)

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#newcode134
    misc/emacs/go-mode.el:134: (defun go--backward-irrelevant (&optional
    stop-at-string)
    On 2013/02/21 18:45:49, adonovan wrote:
    What's the significance of two dashes in go--foo? (Private?)
    Yup, private.

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#newcode654
    misc/emacs/go-mode.el:654: (defun go-root-and-paths ()
    On 2013/02/21 18:45:49, adonovan wrote:
    I recommend adding docstrings even for short internal functions.
    I'll sneak that in in a future CL. I really want this CL to settle down
    so it can be accepted without needing constant reevaluation.

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#newcode679
    misc/emacs/go-mode.el:679: ;; FIXME Technically, -o /dev/null fails in
    quite some cases (on
    On 2013/02/21 18:45:49, adonovan wrote:
    Prefer "TODO($USER): fix: ..." to "FIXME: ...".
    Same as above, in a later CL.

    https://codereview.appspot.com/7314113/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Adonovan at Feb 21, 2013 at 7:14 pm
    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (left):

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#oldcode111
    misc/emacs/go-mode.el:111: (define-key m "}"
    #'go-mode-insert-and-indent)
    On 2013/02/21 19:08:34, Dominik Honnef wrote:
    On 2013/02/21 18:45:49, adonovan wrote:
    Though #'foo is functionally equivalent to 'foo in Emacs Lisp if foo
    is a
    function, I would still recommend the traditional #' as it both
    serves as
    documentation and enables compiler optimisations (since it indicates
    that the
    name of the symbol isn't significant).
    I actually wasn't too sure about the meaning of #'. Last time I
    researched it,
    it was said that # causes the compiler to inline functions, which in
    my opinion
    didn't make much sense for define-key. I'd really love some
    documentation on #'

    Just as the reader expands 'x to (quote x), it expands #'x to the
    (function x). In most dialects of Lisp, you have to use exactly one or
    the other: (quote x) to get the name of the symbol, and (function x) to
    get the function associated with the symbol. Emacs Lisp's function
    application operator is slightly weird though, so it works given either
    the "name" or the function value.

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (right):

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#newcode120
    misc/emacs/go-mode.el:120: `(car (syntax-ppss)))
    On 2013/02/21 19:08:34, Dominik Honnef wrote:
    On 2013/02/21 18:45:49, adonovan wrote:
    Glad to see you're using ppss; I never understood why the old mode
    avoided
    just
    about the most optimised C function in Emacs.
    Heh, two reasons, I'll start from the back: They ditched the syntax table
    completely to properly handle raw strings, which have no escape
    sequences. The
    syntax table can't handle that. My solution is limited to XEmacs and GNU Emacs
    24.x and degrades gracefully on older versions.
    The other reason is: The authors of the old go-mode thought, and I
    quote, that
    emacs has no built-in way to tell you the current nesting in
    parentheses. Or in
    other words, they probably didn't know syntax-ppss :)
    Ah. And the reviewer was afraid to mention it after they'd reinvented
    the wheel, I suppose. :)

    https://codereview.appspot.com/7314113/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Dominik Honnef at Feb 21, 2013 at 7:27 pm
    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (left):

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#oldcode111
    misc/emacs/go-mode.el:111: (define-key m "}"
    #'go-mode-insert-and-indent)
    On 2013/02/21 19:14:40, adonovan wrote:
    On 2013/02/21 19:08:34, Dominik Honnef wrote:
    On 2013/02/21 18:45:49, adonovan wrote:
    Though #'foo is functionally equivalent to 'foo in Emacs Lisp if
    foo is a
    function, I would still recommend the traditional #' as it both
    serves as
    documentation and enables compiler optimisations (since it
    indicates that
    the
    name of the symbol isn't significant).
    I actually wasn't too sure about the meaning of #'. Last time I
    researched it,
    it was said that # causes the compiler to inline functions, which in
    my
    opinion
    didn't make much sense for define-key. I'd really love some
    documentation on
    #'
    Just as the reader expands 'x to (quote x), it expands #'x to the
    (function x).
    In most dialects of Lisp, you have to use exactly one or the other:
    (quote x) to
    get the name of the symbol, and (function x) to get the function
    associated with
    the symbol. Emacs Lisp's function application operator is slightly weird
    though, so it works given either the "name" or the function value.
    Ah. Thanks for explaining that, that cleared it up.

    Looking through the packages that come with GNU Emacs, I can see that
    there's no consensus on whether to use #' or not. I'll delay this until
    a future CL, for the same reason as before.

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el
    File misc/emacs/go-mode.el (right):

    https://codereview.appspot.com/7314113/diff/23002/misc/emacs/go-mode.el#newcode120
    misc/emacs/go-mode.el:120: `(car (syntax-ppss)))
    On 2013/02/21 19:14:40, adonovan wrote:
    On 2013/02/21 19:08:34, Dominik Honnef wrote:
    On 2013/02/21 18:45:49, adonovan wrote:
    Glad to see you're using ppss; I never understood why the old mode
    avoided
    just
    about the most optimised C function in Emacs.
    Heh, two reasons, I'll start from the back: They ditched the syntax
    table
    completely to properly handle raw strings, which have no escape
    sequences. The
    syntax table can't handle that. My solution is limited to XEmacs and
    GNU Emacs
    24.x and degrades gracefully on older versions.

    The other reason is: The authors of the old go-mode thought, and I
    quote, that
    emacs has no built-in way to tell you the current nesting in
    parentheses. Or
    in
    other words, they probably didn't know syntax-ppss :)
    Ah. And the reviewer was afraid to mention it after they'd reinvented the
    wheel, I suppose. :)
    Well, even the original version of the old mode implemented its own
    parser for some parts, so it was only made worse. There aren't that many
    gophers who use Emacs, and this CL already had more reviews than all
    other go-mode related CLs combined. Most of the previous changes just
    got accepted without a real review process.

    https://codereview.appspot.com/7314113/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedFeb 21, '13 at 12:33a
activeFeb 21, '13 at 7:27p
posts9
users4
websitegolang.org

People

Translate

site design / logo © 2021 Grokbase