478524 - Support for adding arbitrary "Markers" in the line number gutter

Status: VERIFIED FIXED

(Skywriter Graveyard :: Editor, enhancement, P4)

Description

[Simon Kaegi]

User-Agent:       Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 2.0.50727; .NET CLR 3.0.04506.30; .NET CLR 1.1.4322; .NET CLR 3.0.04506.648; .NET CLR 3.0.4506.2152; .NET CLR 3.5.30729)
Build Identifier: 

For example support for adding an error or warning marker to indicate there is a syntax error on the line.

Reproducible: Always

Comment 1

[Gordon P. Hemsley [:GPHemsley]]

Confirming, because this would be a useful feature.

I would like to note, though, that there already is a syntax notifier that pops up a little window to annoy you when you've typed something with incorrect syntax.

Comment 2

[Chris J]

Attachment: Initial patch to add lineMarkers

Initial patch to add lineMarkers. When a parser error occurs, the gutter changes color according to new lineMarkerGutterColor and lineMarkerFontColor theme options. You can click on the gutter to see the parser error message again.

Although the parser currently only ever reports a single error, the code is written such that multiple lineMarkers can be present simultaneously. This should enable future enhancements such as jslint integration. 

As this is my first ever patch to any Mozilla product, please let me know how I'm doing!

Comment 3

[Gordon P. Hemsley [:GPHemsley]]

At a quick glance, the code looks good, though I haven't actually applied it to ensure that it works.

However, I'm assigning this to you, Chris, so you can do any follow-up changes recommended by those more familiar with this area.

Comment 4

[Chris J]

Attachment: Rebased to tip

There have been some changes to editor.js since I originally wrote the patch. Updated to fit latest trunk as of 13-Apr-09.

Comment 5

[Dion Almaer]

Thanks Chris. I have added Ben to this puppy as he was just in there doing the smart gutter work and I want him to review.

Comment 6

[Ben Galbraith]

Chris, I like this patch, thanks for your work on it. I've applied it.

Comment 7

[Gordon P. Hemsley [:GPHemsley]]

Ben applied this patch in changeset e0bd354779b2. However, as far as I know, neither he nor I have been able to actually add markers in the gutter. Are we missing something? If so, is this fixed? Can it be closed?

Comment 8

[Chris J]

Uh - the only way to add markers to the gutter is to create a parser error in javascript, e.g. try typing "deliberate() error()" in the editor. If you do that, the gutter should light up against the relevant line. Once the command line message has gone, you can resurrect it by clicking on the gutter highlight.

I'm currently working on bug 486949 to add jslint support which will add a whole lot more line markers as it notices things like missing semicolons.

Comment 9

[Chris J]

Attachment: interdiff with added themes to make lineMarkers visible

Well it turns out my patch relied on themes.js which has changed. Here is the new patch, it only modifies the themes files because the rest of the code has been checked in and works fine.

Let me know how this goes, when you apply this patch it should highlight the gutter when there's an error.

Comment 10

[Dion Almaer]

Chris,

Thanks! Added this info to the styles in change set e647724cac9f.

Good to go?

Comment 11

[Gordon P. Hemsley [:GPHemsley]]

This is a mass migration from Mozilla Labs :: Bespin to Bespin :: Editor.

Comment 12

[Chris J]

Attachment: Adds jslint function messages, simplifies logic

Applies to tip. This patch simplifies the lineMarkers code and adds a new "messages" type, alongside "errors" and "warnings". It appears as a green gutter marking against each function in a javascript file. If you click on it, it displays the function closures, variables, unused variables, etc, all from jslint.

Comment 13

[Chris J]

One more point - to see this, open a javascript file and ensure you have "set syntaxcheck all" or "set syntaxcheck warnings". 

The possible values of the "syntaxcheck" setting are:
* all (see errors, warnings and messages
* message (see messages)
* warning (see warnings)
* error (see errors)

Comment 14

[Chris J]

Attachment: v2 of messages patch

Well actually the old "set syntaxcheck errors" stuff has been removed on tip to prepare for code complete ... so I added a new setting

"set syntaxmarkers" which has the possible values
* "all": show all markers (errors, warnings and messages)
* "errors": show just the error markers
* "warnings": show just the warning markers
* "messages": show just the message markers
* "none": don't show any markers

this works alongside syntaxcheck, obviously if syntaxcheck is off then none of the markers show anyway

Comment 15

[Chris J]

code is now slightly out of date, revised patch coming.

Comment 16

[Chris J]

Attachment: rebased to new code folding patch

Rebased to match the new code folding patch in bug 473329. It also fixes a very annoying bug where the parser doesn't start automatically when you load the document.

Comment 17

[Chris J]

Attachment: Rebased to tip

Rebased to tip, this is read to apply.

Comment 18

[Chris J]

Attachment: rebased to tip, default syntaxmarkers setting and fix parse rerun issue

Rebased to tip.
Fixes an issue with the parser where it doesn't run if the document changes
Adds a default setting "syntaxmarkers" as described above

Comment 19

[Ben Galbraith]

Committed patch; can I close the issue?

Comment 20

[Chris J]

pushed to tip, this resolves the issue. Thanks Ben.

Comment 21

[Gordon P. Hemsley [:GPHemsley]]

This was committed in changeset 68efb9bef23f and does appear to work. Marking as VERIFIED.