Discussion:
code review 6566045: playground: Adding line numbers and error highlighting. (issue 6566045)
(too old to reply)
campoy-iFWiy5xATs8dnm+
2012-09-25 08:18:39 UTC
Permalink
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev-/***@public.gmane.org,

I'd like you to review this change to
https://code.google.com/p/go-playground


Description:
playground: Adding line numbers and error highlighting.

Please review this at http://codereview.appspot.com/6566045/

Affected files:
M goplay/edit.html
A static/jquery-linedtextarea.js
M static/playground.js
M static/style.css
campoy-iFWiy5xATs8dnm+
2012-09-25 08:22:42 UTC
Permalink
Hello golang-dev-/***@public.gmane.org (cc: golang-dev-/***@public.gmane.org),

Please take another look.


http://codereview.appspot.com/6566045/
campoy-iFWiy5xATs8dnm+
2012-09-25 08:41:38 UTC
Permalink
Hello adg-iFWiy5xATs8dnm+***@public.gmane.org (cc: golang-dev-/***@public.gmane.org),

Please take another look.


http://codereview.appspot.com/6566045/
Rob Pike
2012-09-25 08:56:19 UTC
Permalink
Received: by 10.52.30.133 with SMTP id s5mr2276758vdh.10.1348563381714;
Tue, 25 Sep 2012 01:56:21 -0700 (PDT)
X-BeenThere: golang-dev-/***@public.gmane.org
Received: by 10.220.149.129 with SMTP id t1ls4781699vcv.0.gmail; Tue, 25 Sep
2012 01:56:20 -0700 (PDT)
Received: by 10.52.67.38 with SMTP id k6mr3281280vdt.2.1348563380943;
Tue, 25 Sep 2012 01:56:20 -0700 (PDT)
Received: by 10.52.67.38 with SMTP id k6mr3281279vdt.2.1348563380935;
Tue, 25 Sep 2012 01:56:20 -0700 (PDT)
Received: from mail-vb0-f53.google.com (mail-vb0-f53.google.com [209.85.212.53])
by gmr-mx.google.com with ESMTPS id ef10si1025407vdb.3.2012.09.25.01.56.19
(version=TLSv1/SSLv3 cipher=OTHER);
Tue, 25 Sep 2012 01:56:19 -0700 (PDT)
Received-SPF: pass (google.com: domain of ***@golang.org designates 209.85.212.53 as permitted sender) client-ip=209.85.212.53;
Received: by vbbfc21 with SMTP id fc21so7257042vbb.40
for <golang-dev-/***@public.gmane.org>; Tue, 25 Sep 2012 01:56:19 -0700 (PDT)
Received: by 10.220.148.211 with SMTP id q19mr8925427vcv.55.1348563379728;
Tue, 25 Sep 2012 01:56:19 -0700 (PDT)
Received: by 10.58.238.73 with HTTP; Tue, 25 Sep 2012 01:56:19 -0700 (PDT)
In-Reply-To: <047d7bdc8c52b01fde04ca82ac00-hpIqsD4AKlfQT0dZR+***@public.gmane.org>
X-Gm-Message-State: ALoCoQnvaH6yUSFwquCwzI1/qqztWeoTuU8jaVKZuqnoMD3wmd5svrMvlIXY8jdlrK2yaiEf4Dao
X-Original-Sender: ***@golang.org
X-Original-Authentication-Results: gmr-mx.google.com; spf=pass (google.com:
domain of ***@golang.org designates 209.85.212.53 as permitted sender) smtp.mail=***@golang.org
Precedence: list
Mailing-list: list golang-dev-/***@public.gmane.org; contact golang-dev+owners-/***@public.gmane.org
List-ID: <golang-dev.googlegroups.com>
X-Google-Group-Id: 1097896213209
List-Post: <http://groups.google.com/group/golang-dev/post?hl=en_US>, <mailto:golang-dev-/***@public.gmane.org>
List-Help: <http://groups.google.com/support/?hl=en_US>, <mailto:golang-dev+help-/***@public.gmane.org>
List-Archive: <http://groups.google.com/group/golang-dev?hl=en_US>
Sender: golang-dev-/***@public.gmane.org
List-Subscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:golang-dev+subscribe-/***@public.gmane.org>
List-Unsubscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:googlegroups-manage+1097896213209+unsubscribe-/***@public.gmane.org>
Archived-At: <http://permalink.gmane.org/gmane.comp.lang.go.devel/46761>

Can we see it running somewhere?

-rob
Francesc Campoy Flores
2012-09-25 16:12:20 UTC
Permalink
It's now running on my linux machine : canigo.mtv.corp.google.com:8080.

I can access it from my own machine targetting localhost:8080, but it
doesn't work with the server name ... any ideas? Is the google dns down? is
there a new firewall I didn't know about?

Thanks,
Post by Rob Pike
Can we see it running somewhere?
-rob
--
--
Francesc
Andrew Gerrand
2012-09-25 18:10:47 UTC
Permalink
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=google.com; s=20120113;
h=x-beenthere:received-spf:mime-version:sender:in-reply-to:references
:from:date:message-id:subject:to:cc:x-system-of-record
:x-gm-message-state:x-original-sender
:x-original-authentication-results:precedence:mailing-list:list-id
:x-google-group-id:list-post:list-help:list-archive:list-subscribe
:list-unsubscribe:content-type;
bh=7NIm2k7B7XRN89VTSnpDcg2kdjxFsusFOsEBXUm178s=;
b=BZuPI1Gzr1SU/u7m9KVh97C+aGRuFK54Y02oFZaI4XWylsOL2EocrgqvZUZtU1HVnK
Rk/pL6AOxsrMHxyPegLr8RZznSrBw1Y7AbniU/JYUpb2mda2unejc37YhKo1LqoUCWdK
o44l3Bow2D0+/Gghc/NasOIDsGJ83+2dZsSChv6deqzV1VHvzJCFAkiaAIS9uAQnaK00
437FJ9oeQtCXUiAWPSzmxdBH4m5xLbH7D79U3WahdmyJ1QuwXXA2asA3Lb3lh2RwFAiG
jCJZ98VKs0hsYXPrWQv0QQnOF7CgnCKl/SoEDaUp+wpO
Received: by 10.224.33.136 with SMTP id h8mr4373550qad.1.1348596680518;
Tue, 25 Sep 2012 11:11:20 -0700 (PDT)
X-BeenThere: golang-dev-/***@public.gmane.org
Received: by 10.224.146.194 with SMTP id i2ls1120333qav.6.gmail; Tue, 25 Sep
2012 11:11:19 -0700 (PDT)
Received: by 10.224.223.84 with SMTP id ij20mr13683081qab.5.1348596679025;
Tue, 25 Sep 2012 11:11:19 -0700 (PDT)
Received: by 10.224.223.84 with SMTP id ij20mr13683078qab.5.1348596679012;
Tue, 25 Sep 2012 11:11:19 -0700 (PDT)
Received: from mail-qa0-f42.google.com (mail-qa0-f42.google.com [209.85.216.42])
by gmr-mx.google.com with ESMTPS id t6si397723qcp.0.2012.09.25.11.11.17
(version=TLSv1/SSLv3 cipher=OTHER);
Tue, 25 Sep 2012 11:11:18 -0700 (PDT)
Received-SPF: pass (google.com: domain of adg-hpIqsD4AKlfQT0dZR+***@public.gmane.org designates 209.85.216.42 as permitted sender) client-ip=209.85.216.42;
Received: by mail-qa0-f42.google.com with SMTP id t11so2704524qaa.1
for <golang-dev-/***@public.gmane.org>; Tue, 25 Sep 2012 11:11:17 -0700 (PDT)
Received: by 10.224.175.204 with SMTP id bb12mr41733725qab.14.1348596677872;
Tue, 25 Sep 2012 11:11:17 -0700 (PDT)
Sender: golang-dev-/***@public.gmane.org
Received: by 10.224.174.212 with HTTP; Tue, 25 Sep 2012 11:10:47 -0700 (PDT)
In-Reply-To: <CAGUpi4Jr1ZgMkHLp9FM_mGcDdfE3XqT8UFh6oCt2x=jWSNoWSQ-JsoAwUIsXosN+***@public.gmane.org>
X-System-Of-Record: true
X-Gm-Message-State: ALoCoQlQqbR4BD2ZsvQLg19LH60bvjmf3QS3UrteFMR9l5kyP2zA7bW4mHnxyyDffzltv3i0yRNLAl1Vl0kw78vnQQkb5Z9GHzn2eLo9M/ojnb1ULqbKvPakV5EsxWNP0cNTir55RX7DZm7ElXQa0x1ca+z4nKqUDSi9pau0BujGasHnxJn3bGbh3DLcIWP35ZKsAkvMKoUTTO1VJ1KHBgN26wB1w2l5AQ==
X-Original-Sender: adg-hpIqsD4AKlfQT0dZR+***@public.gmane.org
X-Original-Authentication-Results: gmr-mx.google.com; spf=pass (google.com:
domain of adg-hpIqsD4AKlfQT0dZR+***@public.gmane.org designates 209.85.216.42 as permitted sender)
smtp.mail=adg-hpIqsD4AKlfQT0dZR+***@public.gmane.org; dkim=pass header.i=@google.com
Precedence: list
Mailing-list: list golang-dev-/***@public.gmane.org; contact golang-dev+owners-/***@public.gmane.org
List-ID: <golang-dev.googlegroups.com>
X-Google-Group-Id: 1097896213209
List-Post: <http://groups.google.com/group/golang-dev/post?hl=en_US>, <mailto:golang-dev-/***@public.gmane.org>
List-Help: <http://groups.google.com/support/?hl=en_US>, <mailto:golang-dev+help-/***@public.gmane.org>
List-Archive: <http://groups.google.com/group/golang-dev?hl=en_US>
List-Subscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:golang-dev+subscribe-/***@public.gmane.org>
List-Unsubscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:googlegroups-manage+1097896213209+unsubscribe-/***@public.gmane.org>
Archived-At: <http://permalink.gmane.org/gmane.comp.lang.go.devel/46781>

Deploy it to an app engine instance somewhere.
Francesc Campoy Flores
2012-09-25 18:34:52 UTC
Permalink
http://goplaybeta.appspot.com/
Post by Andrew Gerrand
Deploy it to an app engine instance somewhere.
--
--
Francesc
Aram Hăvărneanu
2012-09-25 19:57:38 UTC
Permalink
I'd like it to highlight the whole line, rather than only the line number.
Russ Cox
2012-09-25 20:45:38 UTC
Permalink
Received: by 10.229.135.213 with SMTP id o21mr1095083qct.5.1348605941530;
Tue, 25 Sep 2012 13:45:41 -0700 (PDT)
X-BeenThere: golang-dev-/***@public.gmane.org
Received: by 10.224.205.132 with SMTP id fq4ls1412255qab.1.gmail; Tue, 25 Sep
2012 13:45:40 -0700 (PDT)
Received: by 10.224.213.1 with SMTP id gu1mr14015460qab.7.1348605940432;
Tue, 25 Sep 2012 13:45:40 -0700 (PDT)
Received: by 10.224.213.1 with SMTP id gu1mr14015459qab.7.1348605940422;
Tue, 25 Sep 2012 13:45:40 -0700 (PDT)
Received: from mail-qc0-f176.google.com (mail-qc0-f176.google.com [209.85.216.176])
by gmr-mx.google.com with ESMTPS id t6si547836qcp.0.2012.09.25.13.45.39
(version=TLSv1/SSLv3 cipher=OTHER);
Tue, 25 Sep 2012 13:45:39 -0700 (PDT)
Received-SPF: pass (google.com: domain of rsc-iFWiy5xATs8dnm+***@public.gmane.org designates 209.85.216.176 as permitted sender) client-ip=209.85.216.176;
Received: by qcon41 with SMTP id n41so3372903qco.7
for <golang-dev-/***@public.gmane.org>; Tue, 25 Sep 2012 13:45:39 -0700 (PDT)
Received: by 10.224.174.75 with SMTP id s11mr42325681qaz.88.1348605939190;
Tue, 25 Sep 2012 13:45:39 -0700 (PDT)
Received: by 10.49.128.5 with HTTP; Tue, 25 Sep 2012 13:45:38 -0700 (PDT)
In-Reply-To: <CAGUpi4Jr1ZgMkHLp9FM_mGcDdfE3XqT8UFh6oCt2x=jWSNoWSQ-JsoAwUIsXosN+***@public.gmane.org>
X-Gm-Message-State: ALoCoQn0x4wxXYNOr9+wIhTyaAQOhYb3fscsnkw5qylYKqV5KibvrE4PmpWyNHPZDvdrt50Ggfvr
X-Original-Sender: rsc-iFWiy5xATs8dnm+***@public.gmane.org
X-Original-Authentication-Results: gmr-mx.google.com; spf=pass (google.com:
domain of rsc-iFWiy5xATs8dnm+***@public.gmane.org designates 209.85.216.176 as permitted sender) smtp.mail=rsc-iFWiy5xATs8dnm+***@public.gmane.org
Precedence: list
Mailing-list: list golang-dev-/***@public.gmane.org; contact golang-dev+owners-/***@public.gmane.org
List-ID: <golang-dev.googlegroups.com>
X-Google-Group-Id: 1097896213209
List-Post: <http://groups.google.com/group/golang-dev/post?hl=en_US>, <mailto:golang-dev-/***@public.gmane.org>
List-Help: <http://groups.google.com/support/?hl=en_US>, <mailto:golang-dev+help-/***@public.gmane.org>
List-Archive: <http://groups.google.com/group/golang-dev?hl=en_US>
Sender: golang-dev-/***@public.gmane.org
List-Subscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:golang-dev+subscribe-/***@public.gmane.org>
List-Unsubscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:googlegroups-manage+1097896213209+unsubscribe-/***@public.gmane.org>
Archived-At: <http://permalink.gmane.org/gmane.comp.lang.go.devel/46788>

Very nice.

I love how it just highlights the number instead of the whole line.
One of the things that bugged me about the old stuff was that even
once I'd edited the line or inserted a line above it, there was still
this ugly highlight that no longer applied. Highlighting just the line
numbers fixes that, by being relatively unobtrusive. Very nice. It
might be worth putting a few pixels horizontal spacing between the
line numbers and the program text, though. Right now it looks like the
first line of my program is 1package main.

Russ
Rob Pike
2012-09-25 20:47:22 UTC
Permalink
Received: by 10.50.214.1 with SMTP id nw1mr4240340igc.0.1348606045152;
Tue, 25 Sep 2012 13:47:25 -0700 (PDT)
X-BeenThere: golang-dev-/***@public.gmane.org
Received: by 10.50.51.234 with SMTP id n10ls6234479igo.3.canary; Tue, 25 Sep
2012 13:47:24 -0700 (PDT)
Received: by 10.50.169.39 with SMTP id ab7mr5021839igc.4.1348606043605;
Tue, 25 Sep 2012 13:47:23 -0700 (PDT)
Received: by 10.50.169.39 with SMTP id ab7mr5021838igc.4.1348606043596;
Tue, 25 Sep 2012 13:47:23 -0700 (PDT)
Received: from mail-ie0-f178.google.com (mail-ie0-f178.google.com [209.85.223.178])
by gmr-mx.google.com with ESMTPS id dd6si192385igc.0.2012.09.25.13.47.22
(version=TLSv1/SSLv3 cipher=OTHER);
Tue, 25 Sep 2012 13:47:22 -0700 (PDT)
Received-SPF: pass (google.com: domain of ***@golang.org designates 209.85.223.178 as permitted sender) client-ip=209.85.223.178;
Received: by mail-ie0-f178.google.com with SMTP id e11so1222969iej.37
for <golang-dev-/***@public.gmane.org>; Tue, 25 Sep 2012 13:47:22 -0700 (PDT)
Received: by 10.50.217.229 with SMTP id pb5mr9351039igc.28.1348606042312; Tue,
25 Sep 2012 13:47:22 -0700 (PDT)
Received: by 10.50.191.134 with HTTP; Tue, 25 Sep 2012 13:47:22 -0700 (PDT)
In-Reply-To: <CAA8EjDRA0fUcPPEv1jjucmfAifd5VaWznbO0558UVHWqv8dtsw-JsoAwUIsXosN+***@public.gmane.org>
X-Gm-Message-State: ALoCoQl0Hhd6UOctXVr0NoFGPvnjMRGwlSj9Gulb+jSAsvRmumRfUsg/hQyC+GeHV23K0UnDfcV9
X-Original-Sender: ***@golang.org
X-Original-Authentication-Results: gmr-mx.google.com; spf=pass (google.com:
domain of ***@golang.org designates 209.85.223.178 as permitted sender) smtp.mail=***@golang.org
Precedence: list
Mailing-list: list golang-dev-/***@public.gmane.org; contact golang-dev+owners-/***@public.gmane.org
List-ID: <golang-dev.googlegroups.com>
X-Google-Group-Id: 1097896213209
List-Post: <http://groups.google.com/group/golang-dev/post?hl=en_US>, <mailto:golang-dev-/***@public.gmane.org>
List-Help: <http://groups.google.com/support/?hl=en_US>, <mailto:golang-dev+help-/***@public.gmane.org>
List-Archive: <http://groups.google.com/group/golang-dev?hl=en_US>
Sender: golang-dev-/***@public.gmane.org
List-Subscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:golang-dev+subscribe-/***@public.gmane.org>
List-Unsubscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:googlegroups-manage+1097896213209+unsubscribe-/***@public.gmane.org>
Archived-At: <http://permalink.gmane.org/gmane.comp.lang.go.devel/46789>

I agree: the subtle approach is nice. Clear but not intrusive.

-rob
Francesc Campoy Flores
2012-09-25 21:06:25 UTC
Permalink
5px added between line number and line content.

Thanks
Post by Rob Pike
I agree: the subtle approach is nice. Clear but not intrusive.
-rob
--
--
Francesc
Russ Cox
2012-09-25 21:17:12 UTC
Permalink
Received: by 10.52.175.5 with SMTP id bw5mr2747000vdc.16.1348607834579;
Tue, 25 Sep 2012 14:17:14 -0700 (PDT)
X-BeenThere: golang-dev-/***@public.gmane.org
Received: by 10.52.21.40 with SMTP id s8ls633961vde.8.gmail; Tue, 25 Sep 2012
14:17:13 -0700 (PDT)
Received: by 10.59.1.102 with SMTP id bf6mr4548574ved.21.1348607833534;
Tue, 25 Sep 2012 14:17:13 -0700 (PDT)
Received: by 10.59.1.102 with SMTP id bf6mr4548573ved.21.1348607833527;
Tue, 25 Sep 2012 14:17:13 -0700 (PDT)
Received: from mail-vc0-f173.google.com (mail-vc0-f173.google.com [209.85.220.173])
by gmr-mx.google.com with ESMTPS id bn19si79092vdb.0.2012.09.25.14.17.12
(version=TLSv1/SSLv3 cipher=OTHER);
Tue, 25 Sep 2012 14:17:12 -0700 (PDT)
Received-SPF: pass (google.com: domain of rsc-iFWiy5xATs8dnm+***@public.gmane.org designates 209.85.220.173 as permitted sender) client-ip=209.85.220.173;
Received: by vcbfl15 with SMTP id fl15so9901878vcb.18
for <golang-dev-/***@public.gmane.org>; Tue, 25 Sep 2012 14:17:12 -0700 (PDT)
Received: by 10.220.214.208 with SMTP id hb16mr9854359vcb.56.1348607832375;
Tue, 25 Sep 2012 14:17:12 -0700 (PDT)
Received: by 10.58.114.200 with HTTP; Tue, 25 Sep 2012 14:17:12 -0700 (PDT)
In-Reply-To: <CAGUpi4JO9m=8gPhL1UQu+o7dVKHM71ak=OAWiDx4c6W2geznCw-JsoAwUIsXosN+***@public.gmane.org>
X-Gm-Message-State: ALoCoQlP7KSLQp+Y/OUIZFwjaD+Ylv1zpTe9rEtdcLaOaCQTd5EmteOzs871Zgvq8871KUI37J4e
X-Original-Sender: rsc-iFWiy5xATs8dnm+***@public.gmane.org
X-Original-Authentication-Results: gmr-mx.google.com; spf=pass (google.com:
domain of rsc-iFWiy5xATs8dnm+***@public.gmane.org designates 209.85.220.173 as permitted sender) smtp.mail=rsc-iFWiy5xATs8dnm+***@public.gmane.org
Precedence: list
Mailing-list: list golang-dev-/***@public.gmane.org; contact golang-dev+owners-/***@public.gmane.org
List-ID: <golang-dev.googlegroups.com>
X-Google-Group-Id: 1097896213209
List-Post: <http://groups.google.com/group/golang-dev/post?hl=en_US>, <mailto:golang-dev-/***@public.gmane.org>
List-Help: <http://groups.google.com/support/?hl=en_US>, <mailto:golang-dev+help-/***@public.gmane.org>
List-Archive: <http://groups.google.com/group/golang-dev?hl=en_US>
Sender: golang-dev-/***@public.gmane.org
List-Subscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:golang-dev+subscribe-/***@public.gmane.org>
List-Unsubscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:googlegroups-manage+1097896213209+unsubscribe-/***@public.gmane.org>
Archived-At: <http://permalink.gmane.org/gmane.comp.lang.go.devel/46791>

looks fine. leaving for adg. thanks!
Jan Mercl
2012-09-26 08:28:41 UTC
Permalink
(Admittedly nitpicking, sorry.)

Emphasizing should be done IMO preferably only once. Like: Making a
word in a sentence bold or underlined should carry an additional and
useful information. Making the same word both bold and underlined (or
any multiple combination of other options) is a mistake, again IMO.
And making the line number emphasized by its red color, together with
making the background of the same stand out by its changed color, is
similar to that.

It's a matter of personal preferences, anyway - I would probably stick
to the changed background only. Even though that might render the gray
numbers color unusable - didn't tried.

(That might be even good for something else: The contrast of the grey
numbers on the yellow background seems too low to me. I can read it,
but I guess some people cannot.)

-j
Michael Jones
2012-09-26 08:33:17 UTC
Permalink
What you describe is termed ransom note typography by graphic designers.
There is even a meme for that: "If everybody is special, nobody is special."
Post by Jan Mercl
Making the same word both bold and underlined (or
any multiple combination of other options) is a mistake,
Michael T. Jones | Chief Technology Advocate | mtj-hpIqsD4AKlfQT0dZR+***@public.gmane.org | +1
650-335-5765
campoy-iFWiy5xATs8dnm+
2012-09-25 15:55:59 UTC
Permalink
Hello adg-iFWiy5xATs8dnm+***@public.gmane.org, ***@golang.org (cc: golang-dev-/***@public.gmane.org),

Please take another look.


http://codereview.appspot.com/6566045/
d***@public.gmane.org
2012-09-26 08:38:04 UTC
Permalink
This CL acts weirdly whenever i type a _. It shows up as a space until I
press one of the buttons, then it shows up as an underline.

browser is chrome 22.0.1229.79 for linux/amd64

https://codereview.appspot.com/6566045/
roger peppe
2012-09-26 08:53:45 UTC
Permalink
I also like this. One thing - when you've got an error,
the line number is still highlighted even when you've inserted
more text, making it highlight a line that never had an error.
And there's no way to clear the highlighting.

How about when you insert a line, it clears the highlighting
for all line numbers after the inserted line? (the ones that
are now highlighted inappropriately). Alternatively inserting
a line could simply delete all line number highlighting
(it's easy to get it back, by clicking Run agan).
Christoph Hack
2012-09-26 09:46:35 UTC
Permalink
I'm not quite happy with this CL and I think that the previous editor
(CodeMirror) is far superior and better maintained. All issues that I have
seen on this list so far were either caused by an very early, extremely
outdated version of CodeMirror or by some wrong API usage of the very same
editor. I am currently preparing a CL which re-introduces the CodeMirror
editor again. Features include:

- go mode with automatic indentation
- error highlighting (it currently highlights the line, but it would be
also possible to add marker near the line number or style the line numbers
differently)
- optional syntax highlighting (disabled by default *g*)
- line numbering
- common code base (playground, gotour, present)
- fallback: clients that do not support JS will just get a simple <textarea>

You can test out most of this features by visiting tour.golang.org. I'm
also thinking about adding a permanent (cookie based) "switch to textarea"
option, similar to the toggle lineno / syntax highlighting buttons.
However, I am not entirely sure if such an option is necessary, because
I've tested a lot of browsers and I am currently not aware of any glitches
(and CodeMirror also uses a simple transparent <textarea> internally, with
some layers of <div>s behind it). Removing CodeMirror completely is in
my opinion the wrong direction, since it will work fine for nearly all
users and offers a lot of practical features.

Another advantage of CodeMirror is that it supports markers that are moved
with the text. For example, I have an experimental gotour version locally,
that integrates the codewalk functionally directly into the editor, so that
users of the gotour (or other presentations) can work on exercises that
span over multiple slides. I'm not sure if that's a useful feature, but I
think it's good to have the opportunity to implement such things.

I will try to provide a different CL after my exam tomorrow.

-christoph
Francesc Campoy Flores
2012-09-26 16:20:19 UTC
Permalink
Hi gophers,

Christophe:
Your CL sounds great, but take into account that with this CL I'm tackling
a very simple problem: we need line numbers on the playground, now.
Feel free to send your CL for review with the last version of CodeMirror.
I'll be happy to have a look and test it :)

Roger:
I could wipe all the errors as soon as the content of the textarea changes,
but I don't really want to start building an online editor from scratch,
there's a lot of already great editors out there :-) So far, my way of
removing all errors is rerunning, or reformatting. If you still have
errors, you get new ones, if you got rid of them, you won't see any red
lines.

Cheers,
Post by Christoph Hack
I'm not quite happy with this CL and I think that the previous editor
(CodeMirror) is far superior and better maintained. All issues that I have
seen on this list so far were either caused by an very early, extremely
outdated version of CodeMirror or by some wrong API usage of the very same
editor. I am currently preparing a CL which re-introduces the CodeMirror
- go mode with automatic indentation
- error highlighting (it currently highlights the line, but it would be
also possible to add marker near the line number or style the line numbers
differently)
- optional syntax highlighting (disabled by default *g*)
- line numbering
- common code base (playground, gotour, present)
- fallback: clients that do not support JS will just get a simple <textarea>
You can test out most of this features by visiting tour.golang.org. I'm
also thinking about adding a permanent (cookie based) "switch to textarea"
option, similar to the toggle lineno / syntax highlighting buttons.
However, I am not entirely sure if such an option is necessary, because
I've tested a lot of browsers and I am currently not aware of any glitches
(and CodeMirror also uses a simple transparent <textarea> internally, with
some layers of <div>s behind it). Removing CodeMirror completely is in
my opinion the wrong direction, since it will work fine for nearly all
users and offers a lot of practical features.
Another advantage of CodeMirror is that it supports markers that are moved
with the text. For example, I have an experimental gotour version locally,
that integrates the codewalk functionally directly into the editor, so that
users of the gotour (or other presentations) can work on exercises that
span over multiple slides. I'm not sure if that's a useful feature, but I
think it's good to have the opportunity to implement such things.
I will try to provide a different CL after my exam tomorrow.
-christoph
--
--
Francesc
Andrew Gerrand
2012-09-26 17:37:29 UTC
Permalink
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=google.com; s=20120113;
h=x-beenthere:received-spf:mime-version:sender:in-reply-to:references
:from:date:message-id:subject:to:cc:x-system-of-record
:x-gm-message-state:x-original-sender
:x-original-authentication-results:precedence:mailing-list:list-id
:x-google-group-id:list-post:list-help:list-archive:list-subscribe
:list-unsubscribe:content-type;
bh=4pfJvzEF3cnhLaB5nHZK51yDwA+tx/l2XUvVD7CZrlU=;
b=glF5uHg/srhM6Bjnv4w1qB/9X0WRqmKdHmYsGUk+GO5e7s6sHnJFL00Qc+bxhI9mpn
paBP7EYnJekHiQsYbMznrhrIe+qU9pGYrww52mtf4pQGdGdCwpNTTJ7qZeGzl4+S/g6V
8dEVANJ2det0WJ+HPK/lzCCxZUjXCaE7OwJwenC0wO/btQwylERnpo5BAZrU84T5Dbz7
It9wwTksKr9QL8Xims1FTcsO0er8OqGPvcDblYT1NAzeZYsX8D43+z6i+sbUV2MVwHqd
1GD+/8j0AL0U2N1FdGNMWTZOpbn/wYmsXoxkh8GSvxSB
Received: by 10.224.209.133 with SMTP id gg5mr673335qab.5.1348681081260;
Wed, 26 Sep 2012 10:38:01 -0700 (PDT)
X-BeenThere: golang-dev-/***@public.gmane.org
Received: by 10.229.178.68 with SMTP id bl4ls2429023qcb.9.gmail; Wed, 26 Sep
2012 10:38:00 -0700 (PDT)
Received: by 10.224.181.83 with SMTP id bx19mr1116116qab.1.1348681080017;
Wed, 26 Sep 2012 10:38:00 -0700 (PDT)
Received: by 10.224.181.83 with SMTP id bx19mr1116115qab.1.1348681080007;
Wed, 26 Sep 2012 10:38:00 -0700 (PDT)
Received: from mail-qa0-f42.google.com (mail-qa0-f42.google.com [209.85.216.42])
by gmr-mx.google.com with ESMTPS id a27si324971qck.3.2012.09.26.10.37.59
(version=TLSv1/SSLv3 cipher=OTHER);
Wed, 26 Sep 2012 10:38:00 -0700 (PDT)
Received-SPF: pass (google.com: domain of adg-hpIqsD4AKlfQT0dZR+***@public.gmane.org designates 209.85.216.42 as permitted sender) client-ip=209.85.216.42;
Received: by qaat11 with SMTP id t11so158701qaa.15
for <golang-dev-/***@public.gmane.org>; Wed, 26 Sep 2012 10:37:59 -0700 (PDT)
Received: by 10.224.182.147 with SMTP id cc19mr1904928qab.44.1348681079796;
Wed, 26 Sep 2012 10:37:59 -0700 (PDT)
Sender: golang-dev-/***@public.gmane.org
Received: by 10.224.174.212 with HTTP; Wed, 26 Sep 2012 10:37:29 -0700 (PDT)
In-Reply-To: <CAGUpi4+MyNsKNT1g135-V1BgWZ7uVQnamfJCvk8nL8SS+151Qw-JsoAwUIsXosN+***@public.gmane.org>
X-System-Of-Record: true
X-Gm-Message-State: ALoCoQns/Hnw9jMe6290l46LdBVWy1lLdDWl+lbU+d8Ot0iwTOZ29yK3TcX/77o/F3dQ/ON3FH5y9sf7jkkqIIIqJIleDNk8N3mR/9aH4GHE4/nW6jFAF9hM1LvLHgboJ05/PK4YBp7deWO0oDd5olGQuFnQ3f8bt8Bsl4R6iJmTffLAkvIWZdpmKE1wD4XyhrBIeGVqAUBKUvNfT2/6JHoXp3p8gAwuyA==
X-Original-Sender: adg-hpIqsD4AKlfQT0dZR+***@public.gmane.org
X-Original-Authentication-Results: gmr-mx.google.com; spf=pass (google.com:
domain of adg-hpIqsD4AKlfQT0dZR+***@public.gmane.org designates 209.85.216.42 as permitted sender)
smtp.mail=adg-hpIqsD4AKlfQT0dZR+***@public.gmane.org; dkim=pass header.i=@google.com
Precedence: list
Mailing-list: list golang-dev-/***@public.gmane.org; contact golang-dev+owners-/***@public.gmane.org
List-ID: <golang-dev.googlegroups.com>
X-Google-Group-Id: 1097896213209
List-Post: <http://groups.google.com/group/golang-dev/post?hl=en_US>, <mailto:golang-dev-/***@public.gmane.org>
List-Help: <http://groups.google.com/support/?hl=en_US>, <mailto:golang-dev+help-/***@public.gmane.org>
List-Archive: <http://groups.google.com/group/golang-dev?hl=en_US>
List-Subscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:golang-dev+subscribe-/***@public.gmane.org>
List-Unsubscribe: <http://groups.google.com/group/golang-dev/subscribe?hl=en_US>,
<mailto:googlegroups-manage+1097896213209+unsubscribe-/***@public.gmane.org>
Archived-At: <http://permalink.gmane.org/gmane.comp.lang.go.devel/46865>
Post by Francesc Campoy Flores
Hi gophers,
Your CL sounds great, but take into account that with this CL I'm tackling a
very simple problem: we need line numbers on the playground, now.
Feel free to send your CL for review with the last version of CodeMirror.
I'll be happy to have a look and test it :)
I could wipe all the errors as soon as the content of the textarea changes,
but I don't really want to start building an online editor from scratch,
there's a lot of already great editors out there :-) So far, my way of
removing all errors is rerunning, or reformatting. If you still have errors,
you get new ones, if you got rid of them, you won't see any red lines.
I agree on both counts.
Francesc Campoy Flores
2012-09-26 16:13:52 UTC
Permalink
Hi Daniel,

This is really surprising since I'm not playing with the behavior of the
textarea, which keeps being the standard textarea that you can now use on
play.golang.org.

Could you confirm you don't have the same problem with the current version?

Thanks
Post by d***@public.gmane.org
This CL acts weirdly whenever i type a _. It shows up as a space until I
press one of the buttons, then it shows up as an underline.
browser is chrome 22.0.1229.79 for linux/amd64
https://codereview.appspot.**com/6566045/<https://codereview.appspot.com/6566045/>
--
--
Francesc
Francesc Campoy Flores
2012-09-26 19:12:50 UTC
Permalink
The error Daniel mentioned is now fixed, apparently not having Menlo
installed may cause some problems that were not handled correctly by the
"monospace" family.

I added Courier New (which is also monospaced) to the font list to avoid
this problem and now it should be working fine for everybody, let me know
if anyone has an issue with this.

Thanks Daniel for helping debugging this! :-)

Cheers,
Post by d***@public.gmane.org
This CL acts weirdly whenever i type a _. It shows up as a space until I
press one of the buttons, then it shows up as an underline.
browser is chrome 22.0.1229.79 for linux/amd64
https://codereview.appspot.**com/6566045/<https://codereview.appspot.com/6566045/>
--
--
Francesc
d***@public.gmane.org
2012-09-26 16:19:11 UTC
Permalink
Post by Francesc Campoy Flores
Hi Daniel,
This is really surprising since I'm not playing with the behavior of
the
Post by Francesc Campoy Flores
textarea, which keeps being the standard textarea that you can now use
on
Post by Francesc Campoy Flores
http://play.golang.org.
Could you confirm you don't have the same problem with the current
version?
Post by Francesc Campoy Flores
Thanks
Old version works fine. I had some people try the new version with
similar setups in #go-nuts and they couldn't reproduce.

https://codereview.appspot.com/6566045/
adg-iFWiy5xATs8dnm+
2012-09-26 17:50:27 UTC
Permalink
http://codereview.appspot.com/6566045/diff/11001/static/playground.js
File static/playground.js (right):

http://codereview.appspot.com/6566045/diff/11001/static/playground.js#newcode90
static/playground.js:90: regex = /prog.go:([0-9]+)/g
You forgot the var for regex and r, so you're setting global variables.

http://codereview.appspot.com/6566045/diff/11001/static/playground.js#newcode91
static/playground.js:91: for (r = regex.exec(text); r != null; r =
regex.exec(text)) {
var r;
while ((r = regex.exec(text)) != null) {
...
}

http://codereview.appspot.com/6566045/diff/11001/static/playground.js#newcode92
static/playground.js:92:
$('.lineno:nth-child('+r[1]+')').addClass('lineerror')
semicolon

http://codereview.appspot.com/6566045/
campoy-iFWiy5xATs8dnm+
2012-09-26 19:03:55 UTC
Permalink
Hello adg-iFWiy5xATs8dnm+***@public.gmane.org, ***@golang.org, aram-***@public.gmane.org, rsc-iFWiy5xATs8dnm+***@public.gmane.org,
daniel.morsing-***@public.gmane.org, rogpeppe-***@public.gmane.org, tux21b-***@public.gmane.org (cc:
golang-dev-/***@public.gmane.org),

Please take another look.


http://codereview.appspot.com/6566045/
adg-iFWiy5xATs8dnm+
2012-09-26 22:00:34 UTC
Permalink
http://codereview.appspot.com/6566045/diff/5006/static/playground.js
File static/playground.js (right):

http://codereview.appspot.com/6566045/diff/5006/static/playground.js#newcode87
static/playground.js:87: $('.lineerror').toggleClass('lineerror');
s/toggle/remove/

http://codereview.appspot.com/6566045/diff/5006/static/playground.js#newcode92
static/playground.js:92: for (r = regex.exec(text); r != null; r =
regex.exec(text)) {
please:
while ((r = regex.exec(text)) != null) {

http://codereview.appspot.com/6566045/diff/5006/static/playground.js#newcode93
static/playground.js:93:
$('.lineno:nth-child('+r[1]+')').addClass('lineerror');
$(".lineno").eq(r[1]).addClass("lineerror");

note double quotes

http://codereview.appspot.com/6566045/diff/5006/static/style.css
File static/style.css (left):

http://codereview.appspot.com/6566045/diff/5006/static/style.css#oldcode12
static/style.css:12: padding: 5px;
why did you remove the padding?
if you need to introduce a second wrapper element to use the padding
with height:100% then that's fine.

http://codereview.appspot.com/6566045/diff/5006/static/style.css
File static/style.css (right):

http://codereview.appspot.com/6566045/diff/5006/static/style.css#newcode35
static/style.css:35: float: right;
I don't see why the code needs to be float:right if the line numbers are
float:left

http://codereview.appspot.com/6566045/diff/5006/static/style.css#newcode101
static/style.css:101: .lines {
what's the difference between this and ".linedwrap .lines"?

if you just set the background in #wrap - like it used to be - then you
shouldn't need to reset it here.

http://codereview.appspot.com/6566045/
campoy-iFWiy5xATs8dnm+
2012-09-27 06:17:06 UTC
Permalink
Hello adg-iFWiy5xATs8dnm+***@public.gmane.org, ***@golang.org, aram-***@public.gmane.org, rsc-iFWiy5xATs8dnm+***@public.gmane.org,
daniel.morsing-***@public.gmane.org, rogpeppe-***@public.gmane.org, tux21b-***@public.gmane.org (cc:
golang-dev-/***@public.gmane.org),

Please take another look.


http://codereview.appspot.com/6566045/
campoy-iFWiy5xATs8dnm+
2012-09-27 06:21:03 UTC
Permalink
PTAL


http://codereview.appspot.com/6566045/diff/11001/static/playground.js
File static/playground.js (right):

http://codereview.appspot.com/6566045/diff/11001/static/playground.js#newcode90
static/playground.js:90: regex = /prog.go:([0-9]+)/g
Post by adg-iFWiy5xATs8dnm+
You forgot the var for regex and r, so you're setting global
variables.

Done.

http://codereview.appspot.com/6566045/diff/11001/static/playground.js#newcode91
static/playground.js:91: for (r = regex.exec(text); r != null; r =
regex.exec(text)) {
Post by adg-iFWiy5xATs8dnm+
var r;
while ((r = regex.exec(text)) != null) {
...
}
Done.

http://codereview.appspot.com/6566045/diff/11001/static/playground.js#newcode92
static/playground.js:92:
$('.lineno:nth-child('+r[1]+')').addClass('lineerror')
Post by adg-iFWiy5xATs8dnm+
semicolon
Done.

http://codereview.appspot.com/6566045/diff/5006/static/playground.js
File static/playground.js (right):

http://codereview.appspot.com/6566045/diff/5006/static/playground.js#newcode87
static/playground.js:87: $('.lineerror').toggleClass('lineerror');
Post by adg-iFWiy5xATs8dnm+
s/toggle/remove/
Done.

http://codereview.appspot.com/6566045/diff/5006/static/playground.js#newcode92
static/playground.js:92: for (r = regex.exec(text); r != null; r =
regex.exec(text)) {
Post by adg-iFWiy5xATs8dnm+
while ((r = regex.exec(text)) != null) {
Done.

http://codereview.appspot.com/6566045/diff/5006/static/playground.js#newcode93
static/playground.js:93:
$('.lineno:nth-child('+r[1]+')').addClass('lineerror');
Why double quotes here and not before, such as on line 87?
Post by adg-iFWiy5xATs8dnm+
$(".lineno").eq(r[1]).addClass("lineerror");
note double quotes
http://codereview.appspot.com/6566045/diff/5006/static/style.css
File static/style.css (left):

http://codereview.appspot.com/6566045/diff/5006/static/style.css#oldcode12
static/style.css:12: padding: 5px;
Post by adg-iFWiy5xATs8dnm+
why did you remove the padding?
if you need to introduce a second wrapper element to use the padding
with
Post by adg-iFWiy5xATs8dnm+
height:100% then that's fine.
Done.

http://codereview.appspot.com/6566045/diff/5006/static/style.css
File static/style.css (right):

http://codereview.appspot.com/6566045/diff/5006/static/style.css#newcode35
static/style.css:35: float: right;
Post by adg-iFWiy5xATs8dnm+
I don't see why the code needs to be float:right if the line numbers
are
Post by adg-iFWiy5xATs8dnm+
float:left
Avoids mysterious flickering on Chrome.

http://codereview.appspot.com/6566045/diff/5006/static/style.css#newcode101
static/style.css:101: .lines {
Post by adg-iFWiy5xATs8dnm+
what's the difference between this and ".linedwrap .lines"?
if you just set the background in #wrap - like it used to be - then
you
Post by adg-iFWiy5xATs8dnm+
shouldn't need to reset it here.
Structure simplified.

http://codereview.appspot.com/6566045/
adg-iFWiy5xATs8dnm+
2012-09-27 22:03:06 UTC
Permalink
https://codereview.appspot.com/6566045/diff/5006/static/playground.js
File static/playground.js (right):

https://codereview.appspot.com/6566045/diff/5006/static/playground.js#newcode93
static/playground.js:93:
$('.lineno:nth-child('+r[1]+')').addClass('lineerror');
Post by campoy-iFWiy5xATs8dnm+
Why double quotes here and not before, such as on line 87?
I just missed them.

https://codereview.appspot.com/6566045/diff/9008/static/playground.js
File static/playground.js (right):

https://codereview.appspot.com/6566045/diff/9008/static/playground.js#newcode82
static/playground.js:82: '<div class="loading">Waiting for remote
server...</div>'
the reason we don't use double quotes here is because the HTML string
contains double quotes.

https://codereview.appspot.com/6566045/diff/9008/static/playground.js#newcode87
static/playground.js:87: $('.lineerror').removeClass('lineerror');
use double quotes here, too

https://codereview.appspot.com/6566045/
campoy-iFWiy5xATs8dnm+
2012-09-27 22:08:21 UTC
Permalink
Hello adg-iFWiy5xATs8dnm+***@public.gmane.org, ***@golang.org, aram-***@public.gmane.org, rsc-iFWiy5xATs8dnm+***@public.gmane.org,
daniel.morsing-***@public.gmane.org, rogpeppe-***@public.gmane.org, tux21b-***@public.gmane.org (cc:
golang-dev-/***@public.gmane.org),

Please take another look.


http://codereview.appspot.com/6566045/
campoy-iFWiy5xATs8dnm+
2012-09-27 22:08:45 UTC
Permalink
https://codereview.appspot.com/6566045/diff/9008/static/playground.js
File static/playground.js (right):

https://codereview.appspot.com/6566045/diff/9008/static/playground.js#newcode82
static/playground.js:82: '<div class="loading">Waiting for remote
server...</div>'
Post by adg-iFWiy5xATs8dnm+
the reason we don't use double quotes here is because the HTML string
contains
Post by adg-iFWiy5xATs8dnm+
double quotes.
Done.

https://codereview.appspot.com/6566045/diff/9008/static/playground.js#newcode87
static/playground.js:87: $('.lineerror').removeClass('lineerror');
Post by adg-iFWiy5xATs8dnm+
use double quotes here, too
Done.

https://codereview.appspot.com/6566045/
adg-iFWiy5xATs8dnm+
2012-09-27 22:19:31 UTC
Permalink
LGTM

https://codereview.appspot.com/6566045/
campoy-iFWiy5xATs8dnm+
2012-09-27 22:43:58 UTC
Permalink
*** Submitted as
http://code.google.com/p/go-playground/source/detail?r=cb7adf7e3dd3 ***

playground: Adding line numbers and error highlighting.

R=adg, r, aram, rsc, daniel.morsing, rogpeppe, tux21b
CC=golang-dev
http://codereview.appspot.com/6566045


http://codereview.appspot.com/6566045/

Loading...