Fix a case when extension less file is importing other less files #7230

zaggino · 2014-03-18T12:22:24Z

I wanted to split less file my extension is using into more files, but loading failed when using valid import directives. Through debugging, I found out that some properties of file info were not filled as it should.

These are the values for file brackets.less:

currentDirectory: "file:///D:/Program%20Files/Brackets/dev/src/styles/"
entryPath:        "file:///D:/Program%20Files/Brackets/dev/src/styles/"
filename:         "file:///D:/Program%20Files/Brackets/dev/src/styles/brackets.less"
relativeUrls:     undefined
rootFilename:     "file:///D:/Program%20Files/Brackets/dev/src/styles/brackets.less"
rootpath:         "file:///D:/Program%20Files/Brackets/dev/src/styles/"

And these are the original values calculated for brackets-git.less:

currentDirectory: ""
entryPath:        ""
filename:         "brackets-git.less"
relativeUrls:     undefined
rootFilename:     "brackets-git.less"
rootpath:         "file:///C:/Users/Zaggi/AppData/Roaming/Brackets/extensions/user/zaggino.brackets-git/less/"

The fix makes fileInfo look the same way as in the brackets.less case and all references are loaded correctly.

zaggino · 2014-03-18T12:24:43Z

Btw, is there a reason Brackets is still using less 1.4.2 when latest version is 1.7.0 ? The fix works for both versions and I didn't find any ill effects when upgrading to 1.7.0.

le717 · 2014-03-18T14:11:29Z

@zaggino Discussion about updating Less to 1.7.0 is in #6730.

zaggino · 2014-03-18T19:24:21Z

Thanks @le717, I thought I've seen it somewhere but I wasn't sure. Anyway it's not included in this PR.

redmunds · 2014-03-19T00:50:40Z

There's an Extension Utils unit test failing:

should attach LESS style sheets

Error: Expected '/* basic.less */
#project-title {
  font-size: 66px;
}
' to be '/* basic.less */
#project-title {
  font-weight: 800;
}
#project-title {
  font-size: 66px;
}
'.
    at new jasmine.ExpectationResult (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:102:32)
    at null.toBe (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1194:29)
    at null.<anonymous> (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/spec/ExtensionUtils-test.js:134:40)
    at jasmine.Block.execute (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1024:15)
    at jasmine.Queue.next_ (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1842:31)
    at onComplete (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1838:18)
    at jasmine.WaitsForBlock.execute (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:2322:5)
    at file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:2336:12

zaggino · 2014-03-19T01:30:36Z

@redmunds It seems my issue only relates to absolute url's. Do we have any function like FileSystem.isAbsolutePath which checks if url starts with (http|https|file):// ?

zaggino · 2014-03-19T01:34:33Z

I'd probably put the isAbsoluteUrl function into StringUtils.js and use that instead of file:// if there's not a better way.

redmunds · 2014-03-19T01:35:13Z

In thirdparty/path-utils/path-utils.js there's an isAbsoluteUrl function.

zaggino · 2014-03-19T02:03:27Z

Review again please @redmunds

redmunds · 2014-03-19T03:08:06Z

test/spec/ExtensionUtils-test.js

@@ -137,6 +137,36 @@ define(function (require, exports, module) {
                    expect(testWindow.$.contains(testWindow.document, result)).toBeTruthy();
                });
            });
+
+            // putting everything LESS related in 1 test so it runs faster


This comment doesn't seems to make sense. Remove both copies.

redmunds · 2014-03-19T03:08:42Z

Looks good. Just 1 minor comment.

zaggino · 2014-03-19T03:22:17Z

Done, comments removed.

redmunds · 2014-03-19T03:32:43Z

Looks good. Thanks for unit test! Merging.

Fix a case when extension less file is importing other less files

Fix a case when extension less file is importing other less files

cfb4bf4

redmunds self-assigned this Mar 18, 2014

Unit test no longer fails

5433de5

Use isAbsoluteUrl and add passing test for absolute paths

8568b20

redmunds reviewed Mar 19, 2014
View reviewed changes

Removed comments

ec54316

redmunds added a commit that referenced this pull request Mar 19, 2014

Merge pull request #7230 from adobe/zaggino/less-fix

0e8b080

Fix a case when extension less file is importing other less files

redmunds merged commit 0e8b080 into master Mar 19, 2014

redmunds deleted the zaggino/less-fix branch March 19, 2014 03:32

zaggino mentioned this pull request Mar 20, 2014

Use marked.js to show changelog in Markdown mode brackets-userland/brackets-git#253

Merged

zaggino mentioned this pull request Mar 28, 2014

Added Git-FTP init support. brackets-userland/brackets-git#308

Merged

This was referenced Apr 14, 2014

Strange UI regression brackets-userland/brackets-git#396

Closed

less imports loading is still broken on linux and mac #7513

Closed

core-ai-bot mentioned this pull request Aug 30, 2021

[CLOSED] Fix a case when extension less file is importing other less files brackets-archive/bracketsIssues#6535

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix a case when extension less file is importing other less files #7230

Fix a case when extension less file is importing other less files #7230

Uh oh!

zaggino commented Mar 18, 2014

Uh oh!

zaggino commented Mar 18, 2014

Uh oh!

le717 commented Mar 18, 2014

Uh oh!

zaggino commented Mar 18, 2014

Uh oh!

redmunds commented Mar 19, 2014

Uh oh!

zaggino commented Mar 19, 2014

Uh oh!

zaggino commented Mar 19, 2014

Uh oh!

redmunds commented Mar 19, 2014

Uh oh!

zaggino commented Mar 19, 2014

Uh oh!

redmunds Mar 19, 2014

Uh oh!

redmunds commented Mar 19, 2014

Uh oh!

zaggino commented Mar 19, 2014

Uh oh!

redmunds commented Mar 19, 2014

Uh oh!

Uh oh!

Fix a case when extension less file is importing other less files #7230

Fix a case when extension less file is importing other less files #7230

Uh oh!

Conversation

zaggino commented Mar 18, 2014

Uh oh!

zaggino commented Mar 18, 2014

Uh oh!

le717 commented Mar 18, 2014

Uh oh!

zaggino commented Mar 18, 2014

Uh oh!

redmunds commented Mar 19, 2014

Uh oh!

zaggino commented Mar 19, 2014

Uh oh!

zaggino commented Mar 19, 2014

Uh oh!

redmunds commented Mar 19, 2014

Uh oh!

zaggino commented Mar 19, 2014

Uh oh!

redmunds Mar 19, 2014

Choose a reason for hiding this comment

Uh oh!

redmunds commented Mar 19, 2014

Uh oh!

zaggino commented Mar 19, 2014

Uh oh!

redmunds commented Mar 19, 2014

Uh oh!

Uh oh!