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] All source mixes quote styles #1092

Open
core-ai-bot opened this issue Aug 29, 2021 · 14 comments
Open

[CLOSED] All source mixes quote styles #1092

core-ai-bot opened this issue Aug 29, 2021 · 14 comments

Comments

@core-ai-bot
Copy link
Member

Issue by rwaldron
Thursday Jun 21, 2012 at 20:47 GMT
Originally opened as adobe/brackets#1100


Brackets source code should consistently use double quotes for JS string literals and HTML attribute values. With one exception: if the code is inside a string literal (which already uses double quotes), use single quotes instead to avoid escaping.

The current codebase mostly follows these styles, but it's inconsistent in a few places.

Examples:

// JavaScript
var foo = "some text";
var htmlCode = "<div id='some-id' class='some-class'></div>";

<!-- HTML -->
<div id="some-id" class="some-class"></div>

original description:
I'm not advocating for either, but perhaps a definitive decision to use either single or double throughout the entire code base would be beneficial for maintainability.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Thursday Jun 21, 2012 at 20:54 GMT


Per our style guide all JS code should be using double quotes: https://github.com/adobe/brackets/wiki/Brackets%20Coding%20Conventions

@core-ai-bot
Copy link
Member Author

Comment by rwaldron
Thursday Jun 21, 2012 at 20:59 GMT


Neat - style guides are boss :)

@core-ai-bot
Copy link
Member Author

Comment by pthiess
Friday Jun 22, 2012 at 17:46 GMT


Reviewed - we should use double quotes everywhere - leave it unassigned for now.

@core-ai-bot
Copy link
Member Author

Comment by glortho
Tuesday Jun 26, 2012 at 01:53 GMT


Does this apply to nested quotes as well? For example, should the single quotes below be converted to escaped double quotes or left as they are?

$("<div style='position:fixed;left:-50px;width:50px;height:50px;overflow:auto;'><div style='width:100px;height:100px;'/></div>")

Same question for these:

this.$htmlContent.append('<div class="shadow top"/>')

As this isn't explicit in the style guide, I personally prefer leaving the first one as is and changing the formatting of the second to match the first, but let me know and I'll make it happen.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Jun 26, 2012 at 04:52 GMT


+1@jedverity

Although note that for HTML code in a real .html file, we do use double quotes -- so basically I think the proposal is that the outermost language in the file uses double quotes, while any language nested inside strings uses single quotes if possible.

@core-ai-bot
Copy link
Member Author

Comment by rwaldron
Tuesday Jun 26, 2012 at 13:31 GMT


FWIW, in the jQuery source and tests we use:

var html = "<div id='single-quotes-for-attributes'></div>";

@core-ai-bot
Copy link
Member Author

Comment by ghost
Wednesday Jun 27, 2012 at 15:35 GMT


Can we just use double quotes on the whole just to make everything look the same and clean?

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Wednesday Jun 27, 2012 at 19:17 GMT


@WesleMPennock: The proposal is to use double quotes everywhere, with just one exception: code nested inside a string literal. That seems more readable than using escaped double quotes. Compare:

var html = "<div id='some-id' class='some-class'></div>";
// vs.
var html = "<div id=\"some-id\" class=\"some-class\"></div>";

Does that seem fair to you, or are you saying you prefer the second form?

@core-ai-bot
Copy link
Member Author

Comment by ghost
Wednesday Jun 27, 2012 at 19:22 GMT


Oh I am sorry I misinterpreted the proposal, I prefer the first it is much more readable.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Wednesday Jun 27, 2012 at 20:33 GMT


Ok, sounds like there's a pretty solid consensus then. I've updated the description at top, and I'll update the style guide to cover the nesting case too.

@core-ai-bot
Copy link
Member Author

Comment by JonathanWolfe
Monday Jul 02, 2012 at 00:43 GMT


I've done this issue with the pull request here (adobe/brackets#1211).

I don't know how to attach a pull request to an issue though.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Monday Jul 09, 2012 at 19:50 GMT


Marking FBNC (fixed but not closed) so filer can verify fix.

@core-ai-bot
Copy link
Member Author

Comment by rwaldron
Monday Jul 09, 2012 at 20:08 GMT


LGTM

(If I come across any deviations, I'll post back here on the closed ticket)

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Monday Aug 06, 2012 at 17:38 GMT


Thanks rwldrn! Closing this one out for now.

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