Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1724)

Issue 7314113: code review 7314113: misc/emacs: Greatly improve go-mode for Emacs. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 1 month ago by Dominik Honnef
Modified:
11 years, 1 month ago
Reviewers:
CC:
golang-dev, albert.strasheim, Sameer Ajmani, cw, arthur, proppy.gmail, adonovan, rsc, bradfitz
Visibility:
Public.

Description

misc/emacs: Greatly improve go-mode for Emacs. The original go-mode is plagued with odd behaviour, lack of behaviour typical to modes in Emacs and bugs. This change rewrites great parts of go-mode (basically only keeping the gofmt and godoc functions). Additionally it adds new features such as manipulating package imports. For more information please see https://groups.google.com/group/golang-nuts/browse_frm/thread/3a9d6dae3369c0b5/1efe65e2f7afb190 Fixes issue 3618. Fixes issue 4240. Fixes issue 4322. Fixes issue 4671. Fixes issue 4726.

Patch Set 1 #

Patch Set 2 : diff -r 981ed43b6f82 https://code.google.com/p/go #

Patch Set 3 : diff -r 0adf91947752 https://code.google.com/p/go #

Patch Set 4 : diff -r 0adf91947752 https://code.google.com/p/go #

Total comments: 4

Patch Set 5 : diff -r e93de8482d59 https://code.google.com/p/go #

Patch Set 6 : diff -r e93de8482d59 https://code.google.com/p/go #

Patch Set 7 : diff -r e93de8482d59 https://code.google.com/p/go #

Patch Set 8 : diff -r e93de8482d59 https://code.google.com/p/go #

Total comments: 40

Patch Set 9 : diff -r e93de8482d59 https://code.google.com/p/go #

Total comments: 14

Patch Set 10 : diff -r e93de8482d59 https://code.google.com/p/go #

Patch Set 11 : diff -r e93de8482d59 https://code.google.com/p/go #

Patch Set 12 : diff -r ba6a123a1ea3 https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+611 lines, -740 lines) Patch
M misc/emacs/go-mode.el View 1 2 3 4 5 6 7 8 9 10 11 8 chunks +544 lines, -719 lines 0 comments Download
M misc/emacs/go-mode-load.el View 1 2 3 4 5 6 7 8 9 1 chunk +67 lines, -21 lines 0 comments Download

Messages

Total messages: 37
Dominik Honnef
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
11 years, 1 month ago (2013-02-16 18:10:03 UTC) #1
albert.strasheim
Hello On 2013/02/16 18:10:03, Dominik Honnef wrote: > Hello mailto:golang-dev@googlegroups.com, > > I'd like you ...
11 years, 1 month ago (2013-02-17 18:54:51 UTC) #2
Dominik Honnef
On 2013/02/17 18:54:51, albert.strasheim wrote: > Spotted two minor issues, don't know if they are ...
11 years, 1 month ago (2013-02-17 19:02:43 UTC) #3
albert.strasheim
Hello On Sun, Feb 17, 2013 at 9:02 PM, <dominik.honnef@gmail.com> wrote: > On 2013/02/17 18:54:51, ...
11 years, 1 month ago (2013-02-18 09:16:05 UTC) #4
Sameer Ajmani
On 2013/02/16 18:10:03, Dominik Honnef wrote: > Hello mailto:golang-dev@googlegroups.com, > > I'd like you to ...
11 years, 1 month ago (2013-02-19 20:39:01 UTC) #5
Dominik Honnef
Hello golang-dev@googlegroups.com, fullung@gmail.com, sameer@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
11 years, 1 month ago (2013-02-20 05:05:11 UTC) #6
Dominik Honnef
The new patch set fixes go-import-add. I'll be working on go-remove-unused-imports with Albert Strasheim soon.
11 years, 1 month ago (2013-02-20 05:06:54 UTC) #7
cw
Looks great so far. One issue I had in 'dropping it in' to replace what ...
11 years, 1 month ago (2013-02-20 06:03:39 UTC) #8
Dominik Honnef
Hello golang-dev@googlegroups.com, fullung@gmail.com, sameer@golang.org, cw@f00f.org (cc: golang-dev@googlegroups.com), Please take another look.
11 years, 1 month ago (2013-02-20 07:32:35 UTC) #9
Dominik Honnef
Changed references from -loads.el to -load.el and fixed go-remove-unused-imports
11 years, 1 month ago (2013-02-20 07:33:03 UTC) #10
cw
LGTM So far it's working quite well for me. Certainly better than what as there ...
11 years, 1 month ago (2013-02-20 19:00:40 UTC) #11
arthur
This is great. Thank you very much for doing this. It will be great to ...
11 years, 1 month ago (2013-02-21 00:29:28 UTC) #12
proppy.gmail
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 ...
11 years, 1 month ago (2013-02-21 10:52:16 UTC) #13
Dominik Honnef
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 ...
11 years, 1 month ago (2013-02-21 16:57:29 UTC) #14
arthur
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 ...
11 years, 1 month ago (2013-02-21 18:08:03 UTC) #15
Dominik Honnef
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: > ...
11 years, 1 month ago (2013-02-21 18:21:33 UTC) #16
adonovan
Dominik, thanks for taking this on. At one point I was tempted to switch go-mode.el ...
11 years, 1 month ago (2013-02-21 18:45:49 UTC) #17
Dominik Honnef
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: ...
11 years, 1 month ago (2013-02-21 19:08:34 UTC) #18
adonovan
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 ...
11 years, 1 month ago (2013-02-21 19:14:40 UTC) #19
Dominik Honnef
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: ...
11 years, 1 month ago (2013-02-21 19:27:37 UTC) #20
Dominik Honnef
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) On 2013/02/21 10:52:16, proppy.gmail wrote: > ...
11 years, 1 month ago (2013-02-21 20:29:01 UTC) #21
Sameer Ajmani
I've been using this version of go-mode for since last week and have had no ...
11 years, 1 month ago (2013-02-25 15:36:50 UTC) #22
Dominik Honnef
On 2013/02/25 15:36:50, Sameer Ajmani wrote: > Dominik, do you know whether this go-mode should ...
11 years, 1 month ago (2013-02-25 15:40:43 UTC) #23
Sameer Ajmani
okay, I'll try this out in xemacs. I'd appreciate any feedback from xemacs users on ...
11 years, 1 month ago (2013-02-25 17:38:58 UTC) #24
Sameer Ajmani
Syntax highlighting bug: In this code: const ( typeSingleton = 0 typeFixedSize = 1 typeVariableSize ...
11 years, 1 month ago (2013-02-25 22:17:55 UTC) #25
Sameer Ajmani
On 2013/02/25 22:17:55, Sameer Ajmani wrote: > Syntax highlighting bug: > In this code: > ...
11 years, 1 month ago (2013-02-26 16:25:23 UTC) #26
Dominik Honnef
It will probably happen with all keywords/language builtins, even though I have no idea why. ...
11 years, 1 month ago (2013-02-26 16:49:57 UTC) #27
rsc
I propose that this has been going on long enough that we should just submit ...
11 years, 1 month ago (2013-02-26 17:11:18 UTC) #28
Sameer Ajmani
This is happening with GNU Emacs, not Xemacs. On Tue, Feb 26, 2013 at 11:49 ...
11 years, 1 month ago (2013-02-26 17:18:41 UTC) #29
adonovan
LGTM That's fine by me. On 26 February 2013 12:11, Russ Cox <rsc@golang.org> wrote: > ...
11 years, 1 month ago (2013-02-26 17:20:11 UTC) #30
Dominik Honnef
On 2013/02/26 17:18:41, Sameer Ajmani wrote: > This is happening with GNU Emacs, not Xemacs. ...
11 years, 1 month ago (2013-02-26 17:25:30 UTC) #31
rsc
On Tue, Feb 26, 2013 at 12:25 PM, <dominik.honnef@gmail.com> wrote: > Then I am afraid ...
11 years, 1 month ago (2013-02-26 17:27:42 UTC) #32
Sameer Ajmani
Ok, I will commit this and report anything I find as new issues. On Tue, ...
11 years, 1 month ago (2013-02-26 17:29:23 UTC) #33
Sameer Ajmani
warning: cannot find dominik.honnef@gmail.com in CONTRIBUTORS Dominik, please complete a CLA as described at golang.org/doc/contribute.html#copyright. ...
11 years, 1 month ago (2013-02-26 17:54:28 UTC) #34
Dominik Honnef
I submitted an Individual CLA, using the electronic form, at the same time I submitted ...
11 years, 1 month ago (2013-02-26 17:56:40 UTC) #35
bradfitz
I've been trying to add you, but our tools are broken. Will keep fighting it. ...
11 years, 1 month ago (2013-02-26 18:07:31 UTC) #36
Sameer Ajmani
11 years, 1 month ago (2013-02-26 18:49:29 UTC) #37
*** Submitted as https://code.google.com/p/go/source/detail?r=6f89256bf255 ***

misc/emacs: Greatly improve go-mode for Emacs.

The original go-mode is plagued with odd behaviour, lack of
behaviour typical to modes in Emacs and bugs.

This change rewrites great parts of go-mode (basically only
keeping the gofmt and godoc functions).

Additionally it adds new features such as manipulating package
imports.

For more information please see
https://groups.google.com/group/golang-nuts/browse_frm/thread/3a9d6dae3369c0b...

Fixes issue 3618.
Fixes issue 4240.
Fixes issue 4322.
Fixes issue 4671.
Fixes issue 4726.

R=golang-dev, fullung, sameer, cw, arthur, proppy, adonovan, rsc, bradfitz
CC=golang-dev
https://codereview.appspot.com/7314113

Committer: Sameer Ajmani <sameer@golang.org>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b