Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CLOSED] Replace uses of doc.url with _server.pathToUrl() #5591

Open
core-ai-bot opened this issue Aug 30, 2021 · 7 comments
Open

[CLOSED] Replace uses of doc.url with _server.pathToUrl() #5591

core-ai-bot opened this issue Aug 30, 2021 · 7 comments

Comments

@core-ai-bot
Copy link
Member

Issue by njx
Wednesday Nov 27, 2013 at 18:33 GMT
Originally opened as adobe/brackets#6126


For #5317. doc.url was an old field that used to get set on the document before we refactored the live dev server management in 1f6b5aa, but there were still a few places referencing it.


njx included the following code: https://github.com/adobe/brackets/pull/6126/commits

@core-ai-bot
Copy link
Member Author

Comment by njx
Wednesday Nov 27, 2013 at 18:33 GMT


To@jasonsanjose since he's already looked at this branch :)

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Wednesday Nov 27, 2013 at 19:28 GMT


Confirmed this fixes the issue with .js files and the out-of-sync status.

#5317 also mentions this is broken for HTML-files from a user server. That bug still exists here. The offending code is in LiveDevelopment _docIsOutOfSync. I have a fix:

    function _docIsOutOfSync(doc) {
        var docClass    = _classForDocument(doc),
            liveDoc     = _server && _server.get(doc.file.fullPath),
            isLiveEditingEnabled = liveDoc && liveDoc.isLiveEditingEnabled();

        return doc.isDirty && !isLiveEditingEnabled;
    }

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Wednesday Nov 27, 2013 at 19:32 GMT


@njx review complete

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Dec 03, 2013 at 01:16 GMT


Ah, I missed that other part of the bug. I'll look at your proposed fix.

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Dec 03, 2013 at 01:42 GMT


I added your fix and added a few unit tests to make sure the dirty dot does show up in the JS case and doesn't show up in the live HTML/CSS cases. I didn't add a test for the user-server-HTML case since I wasn't sure of a good way to do that, and it doesn't seem like as critical a case anyway.

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Dec 03, 2013 at 01:43 GMT


Ready for re-review.

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Tuesday Dec 03, 2013 at 04:05 GMT


Looks good. Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant