Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Update source to adhere to quote style guide. Fixes issue #1100. #1211

Merged
merged 4 commits into from
Jul 9, 2012
Merged

Update source to adhere to quote style guide. Fixes issue #1100. #1211

merged 4 commits into from
Jul 9, 2012

Conversation

JonathanWolfe
Copy link
Contributor

Went through the source while I was watching a LoL stream and updated the source to follow the style guide on quotes.

I did not edit tests though.

@njx
Copy link
Contributor

njx commented Jul 2, 2012

Thanks for doing this! We're actually on vacation this week so will probably pick this up next week. However, one thing I noticed was that the CodeMirror submodule SHA is showing as a diff for some reason. Could you look into this?

@JonathanWolfe
Copy link
Contributor Author

When I cloned the repo there was that an un-pushed commit already there, I must have included it in mine.

@ghost ghost assigned redmunds Jul 9, 2012
.replace(/>/g, '>');
.replace(/&/g, "&")
.replace(/"/g, """)
.replace(/"/g, "'")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Global find/replace error. The first parameter needs to stay the same:

.replace(/'/g, "'")

@redmunds
Copy link
Contributor

redmunds commented Jul 9, 2012

Jonathan, you'll need to sign the Contributor's License Agreement: http://dev.brackets.io/brackets-contributor-license-agreement.html

@@ -141,7 +141,7 @@ define(function (require, exports, module) {
//If this is a fully quoted value, return the whole
//thing regardless of position
if (attrValue.length > 1 &&
(startChar === "'" || startChar === '"') &&
(startChar === "'" || startChar === "\"") &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this case is a valid exception to the rule, so please back out this change.

@JonathanWolfe
Copy link
Contributor Author

I also signed the agreement.

ref = domain + "." + ref unless ref =~ /\./
r = "<a href='##{ref}'>#{ref}</a>" if info["$ref"]
elsif info["enum"]
r = "( #{info["enum"].join " | "} )"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know ruby, but this appears to be nested double-quotes. Do they need to be escaped? Even if that's valid, the coding convention doc says to use single-quotes inside double quotes, so I think it's safest to keep this line of code as it was.

There are several other cases in the ruby code that this comment also applies to, so try to find and fix those also. When you're done I'll do another scan of the code. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know Ruby either, but the below should be right.

write "<section id='toc' class='domain'>", "<h1>Table of Contents</h1>"
@in["domains"].each do |info|
domain = info["domain"]
write "<h3><a href='##{info['domain']}'>#{info['domain']}</a></h3>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line now has nested single-quotes. Restore to original code.

@redmunds
Copy link
Contributor

redmunds commented Jul 9, 2012

Looks good. Thanks!

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

Successfully merging this pull request may close these issues.

3 participants