search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] Added search jslint error with icon #3929

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by drewhamlett Saturday Jun 15, 2013 at 23:30 GMT Originally opened as https://github.com/adobe/brackets/pull/4246

I think this is what you guys wanted as far as layout and hover.

I used the row-highlight class as the hover detection. I didn't see that class used anywhere else but it maybe wise to add a class id to the jslint table.

Thanks

screen shot 2013-06-15 at 7 29 27 pm

drewhamlett included the following code: https://github.com/adobe/brackets/pull/4246/commits

core-ai-bot commented 3 years ago

Comment by drewhamlett Sunday Jun 16, 2013 at 16:34 GMT

There's an issue with opening multiple tabs. Looking into it.

core-ai-bot commented 3 years ago

Comment by njx Wednesday Jun 19, 2013 at 18:41 GMT

Cool, this is looking good overall--just a few code comments. Also, the "i" icon looks a bit big and slightly vertically off-center to me.@larz0 - would it make sense to decrease the size of the "i" icon slightly (see screenshot above)?

core-ai-bot commented 3 years ago

Comment by larz0 Wednesday Jun 19, 2013 at 20:47 GMT

@drewhjava here's the update icon based on@njx's feedback http://cl.ly/0I2b1T3Q1F3F. Thanks~

core-ai-bot commented 3 years ago

Comment by njx Saturday Jun 22, 2013 at 00:17 GMT

@drewhjava - the new icon looks pretty good. Looks like there are still a few comments from the previous review that need addressing. I'm out for the next couple of weeks, so I'm reassigning this to@gruehle - please add a note when you've dealt with the other comments. Thanks.

core-ai-bot commented 3 years ago

Comment by peterflynn Tuesday Jul 09, 2013 at 22:26 GMT

Maybe it's not necessary given how great Google is, but I wonder if we should do anything do avoid polluting the search query with parts of the message that are specific to the user's code. E.g. for the JSLint error "'foo' was used before it was defined" you probably don't want to include 'foo' in the search since it's unlikely anyone else will have the same string there. I think we could accomplish that by looking at .raw instead of .reason and then stripping out all the templating symbols. Maybe it's overkill though... Thoughts?

core-ai-bot commented 3 years ago

Comment by peterflynn Tuesday Jul 09, 2013 at 22:29 GMT

Another thought -- what about linking to http://jslinterrors.com/ ? It looks like its search function gets matches on the unmodified .raw strings (at least if you omit the period at the end of the sentence). Or maybe if that site gets enough love then it will just bubble to the top of all the Google results :-)

core-ai-bot commented 3 years ago

Comment by peterflynn Wednesday May 14, 2014 at 21:23 GMT

@drewhjava Just noticed this when reviewing some old pull requests. Are you still interested in pursuing this?

The linting system now uses a generic UI managed by CodeInspection.js, with JSLint as only one of many potential result providers. So I think we'd need to consider how to change this into a generic way for any provider to specify 'more info' links (or clickable icons in general?).

If you're interested I think we'd be happy to review that change -- but it would be different enough from this patch that I think we'd want to start a new pull request for it. So I'll close this one -- but feel free to open an updated PR any time!

Thanks for your efforts!