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



10 years ago
9 years ago


(Reporter: simon_kaegi, Assigned: chris.f.jay)





(1 attachment, 7 obsolete attachments)



10 years ago
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
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.
Severity: normal → enhancement
Ever confirmed: true
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: editor
Version: unspecified → Trunk

Comment 2

10 years ago
Created attachment 372211 [details] [diff] [review]
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!
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.
Assignee: nobody → chris.f.jay

Comment 4

10 years ago
Created attachment 372401 [details] [diff] [review]
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

10 years ago
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.
Priority: -- → P4
Target Milestone: -- → Future

Comment 6

10 years ago
Chris, I like this patch, thanks for your work on it. I've applied it.
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

10 years ago
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

10 years ago
Created attachment 373173 [details] [diff] [review]
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

10 years ago

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

Good to go?
This is a mass migration from Mozilla Labs :: Bespin to Bespin :: Editor.
Component: Bespin → Editor
Product: Mozilla Labs → Bespin
QA Contact: bespin → editor
Whiteboard: editor

Comment 12

9 years ago
Created attachment 383119 [details] [diff] [review]
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

9 years ago
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

9 years ago
Created attachment 383197 [details] [diff] [review]
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
Attachment #383119 - Attachment is obsolete: true

Comment 15

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


9 years ago
Depends on: 384282


9 years ago
Depends on: 473329
No longer depends on: 384282

Comment 16

9 years ago
Created attachment 384489 [details] [diff] [review]
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.
Attachment #372211 - Attachment is obsolete: true
Attachment #372401 - Attachment is obsolete: true
Attachment #373173 - Attachment is obsolete: true
Attachment #383197 - Attachment is obsolete: true

Comment 17

9 years ago
Created attachment 388533 [details] [diff] [review]
Rebased to tip

Rebased to tip, this is read to apply.
Attachment #384489 - Attachment is obsolete: true

Comment 18

9 years ago
Created attachment 388989 [details] [diff] [review]
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
Attachment #388533 - Attachment is obsolete: true

Comment 19

9 years ago
Committed patch; can I close the issue?

Comment 20

9 years ago
pushed to tip, this resolves the issue. Thanks Ben.
Last Resolved: 9 years ago
Resolution: --- → FIXED
This was committed in changeset 68efb9bef23f and does appear to work. Marking as VERIFIED.
Target Milestone: Future → 0.3.x
You need to log in before you can comment on or make changes to this bug.