Replace the URL in URLCodeHints even if it is the current content of … #11284

marcelgerber · 2015-06-16T16:20:37Z

…the attr

Fixes #11087. Turns out we already do what @redmunds described in his comment, but only if the hinted file name is not equal to the current value.

@redmunds It looks like you added this extra condition on purpose. Do you recall on why you did that? Was there a specific case which this was supposed to fix?

cc @redmunds

redmunds · 2015-06-16T16:26:29Z

It looks like you added this extra condition on purpose. Do you recall on why you did that? Was there a specific case which this was supposed to fix?

I can't remember exactly, but it was probably an attempt to prevent unnecessary work.

marcelgerber · 2015-06-16T16:34:24Z

In this case, this is a safe change. Right now, replaceRange is called either way, so we don't prevent unnecessary work one way or another.

marcelgerber · 2016-05-02T18:35:51Z

@redmunds @nethip This goes down the same road as #12393 and has been around for quite a long time - would love if you could take a look at it as well :)

redmunds · 2016-05-02T20:56:44Z

This seems like the right thing to do, but I haven't test the code.

ficristo · 2016-05-03T13:37:17Z

Seems to fix the issue for me. Maybe add a test?

ficristo · 2016-05-03T13:58:23Z

Spoke too soon: now I see an error on the console:
Uncaught TypeError: Cannot read property 'localState' of undefined
for a class like this

.test {
    color: red;
    background-image: url(index.htm
}

If I try to autocomplete the url (should be index.html) I see the error on the console. Note that should be the last rule of the class.
I didn't try on master.

marcelgerber · 2016-05-03T17:02:06Z

Strange. I can't repro your recipe.

marcelgerber · 2016-05-03T17:49:08Z

Also, I've just added a test case.

ficristo · 2016-05-03T18:21:53Z

Also, I've just added a test case.

Awesome!

Strange. I can't repro your recipe.

Sorry, the test is an HTML file. The following should suffix to reproduce.

<html>
<head>
</head>
<body>
    <style type="text/css">
        .test {
            color: red;
            background-image: url(index.htm
        }
    </style>
</body>
</html>

Sometime I can see the following exception too. It's about localState so to problem should be the same

Exception in 'editorChange' listener on Editor {document: Document, _handleDocumentChange: function, _handleDocumentDeleted: function, _handleDocumentLanguageChanged: function, _doWorkingSetSync: function…} TypeError: Cannot read property 'localState' of undefined TypeError: Cannot read property 'localState' of undefined
    at _getContextState (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/language/CSSUtils.js:95:51)
    at _isInPropValue (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/language/CSSUtils.js:138:21)
    at _getSucceedingPropValues (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/language/CSSUtils.js:355:18)
    at _getRuleInfoStartingFromPropValue (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/language/CSSUtils.js:505:40)
    at Object.getInfoAtPos (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/language/CSSUtils.js:623:20)
    at UrlCodeHints.hasCssHints (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/extensions/default/UrlCodeHints/main.js:293:30)
    at UrlCodeHints.hasHints (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/extensions/default/UrlCodeHints/main.js:267:25)
    at file:///C:/Program%20Files%20(x86)/Brackets/dev/src/editor/CodeHintManager.js:486:31
    at Array.some (native)
    at _beginSession (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/editor/CodeHintManager.js:485:26)

marcelgerber · 2016-05-03T19:31:42Z

@ficristo I see, thanks for catching this!
The bug is unrelated to this PR -> reported as #12399.

ficristo · 2016-07-20T18:49:19Z

Running Extensions \ Url Code Hinting tests I see some failing tests.
Can you take a look?

marcelgerber · 2016-07-22T21:29:04Z

@ficristo I've seen a failure in one run (should hint site root ('/') or something along these lines), but when I then ran the same tests again, they all passed. Looks like an intermittent failure.

ficristo · 2016-07-23T07:40:59Z

I see other test failures but they seem unrelated.
I found (should hint site root ('/') intermittent too.
That said LGTM.

…the attr

marcelgerber · 2016-07-23T09:28:35Z

Thanks for reviewing, @ficristo :)

marcelgerber mentioned this pull request May 3, 2016

CSSUtils cannot gather state in some mixed-HTML-CSS cases #12399

Closed

Marcel Gerber added 2 commits July 23, 2016 11:24

Replace the URL in URLCodeHints even if it is the current content of …

0cc5302

…the attr

Add test case

90c878c

marcelgerber force-pushed the marcel/url-hints-replace branch from 91640c7 to 90c878c Compare July 23, 2016 09:24

marcelgerber merged commit 361e81d into master Jul 23, 2016

marcelgerber deleted the marcel/url-hints-replace branch July 23, 2016 09:27

marcelgerber mentioned this pull request Jul 23, 2016

CodeHinter gets confused completing a pre-populated attribute #11087

Closed

ficristo added this to the Release 1.8 milestone Aug 2, 2016

This was referenced Aug 30, 2021

[CLOSED] Replace the URL in URLCodeHints even if it is the current content of … brackets-archive/bracketsIssues#9845

Open

[CLOSED] CSSUtils cannot gather state in some mixed-HTML-CSS cases brackets-archive/bracketsIssues#10622

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace the URL in URLCodeHints even if it is the current content of … #11284

Replace the URL in URLCodeHints even if it is the current content of … #11284

Uh oh!

marcelgerber commented Jun 16, 2015

Uh oh!

redmunds commented Jun 16, 2015

Uh oh!

marcelgerber commented Jun 16, 2015

Uh oh!

marcelgerber commented May 2, 2016

Uh oh!

redmunds commented May 2, 2016

Uh oh!

ficristo commented May 3, 2016

Uh oh!

ficristo commented May 3, 2016

Uh oh!

marcelgerber commented May 3, 2016

Uh oh!

marcelgerber commented May 3, 2016

Uh oh!

ficristo commented May 3, 2016

Uh oh!

marcelgerber commented May 3, 2016

Uh oh!

ficristo commented Jul 20, 2016

Uh oh!

marcelgerber commented Jul 22, 2016

Uh oh!

ficristo commented Jul 23, 2016

Uh oh!

marcelgerber commented Jul 23, 2016

Uh oh!

Uh oh!

Replace the URL in URLCodeHints even if it is the current content of … #11284

Replace the URL in URLCodeHints even if it is the current content of … #11284

Uh oh!

Conversation

marcelgerber commented Jun 16, 2015

Uh oh!

redmunds commented Jun 16, 2015

Uh oh!

marcelgerber commented Jun 16, 2015

Uh oh!

marcelgerber commented May 2, 2016

Uh oh!

redmunds commented May 2, 2016

Uh oh!

ficristo commented May 3, 2016

Uh oh!

ficristo commented May 3, 2016

Uh oh!

marcelgerber commented May 3, 2016

Uh oh!

marcelgerber commented May 3, 2016

Uh oh!

ficristo commented May 3, 2016

Uh oh!

marcelgerber commented May 3, 2016

Uh oh!

ficristo commented Jul 20, 2016

Uh oh!

marcelgerber commented Jul 22, 2016

Uh oh!

ficristo commented Jul 23, 2016

Uh oh!

marcelgerber commented Jul 23, 2016

Uh oh!

Uh oh!