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

no need to count the usage of RFC2119 terms, just flag their presence #379

Merged
merged 1 commit into from
Dec 18, 2014
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 4 additions & 9 deletions js/core/inlines.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ define(
run: function (conf, doc, cb, msg) {
msg.pub("start", "core/inlines");
doc.normalize();
if (!conf.normativeReferences) { conf.normativeReferences = {} };
if (!conf.informativeReferences) { conf.informativeReferences = {} };
if (!conf.respecRFC2119) { conf.respecRFC2119 = {} };
if (!conf.normativeReferences) conf.normativeReferences = {};
if (!conf.informativeReferences) conf.informativeReferences = {};
if (!conf.respecRFC2119) conf.respecRFC2119 = {};
Copy link
Member

Choose a reason for hiding this comment

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

Implied "{" "}" can lead to sadness :(

Copy link
Member Author

Choose a reason for hiding this comment

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

I've heard that rumour for the past twenty years; I've never seen it materialise.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

So, if we were using a different language and making use of goto, and employing a nonsensical identation scheme, and distracted, and stoned from having used Python too much I agree that it could be a problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

ROTFL! Actually if you go back and look at the original pull request that implemented this change, I think we decided counting them was harmless and might at some point be valuable. I don't really care. And I wrote it. But is there a compelling reason to NOT have this information around?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not a big deal but I was in that code for other reasons and I puzzled at what those six lines were doing. If we need the information we know how to add it but, but otherwise YAGNI. It's just a clean-up patch, I didn't expect the Spanish Inquisition.

Copy link
Contributor

Choose a reason for hiding this comment

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

NO ONE EXPECTS THE SPANISH INQUISITION!

Sorry. I couldn't resist. I will merge it in.


// PRE-PROCESSING
var abbrMap = {}, acroMap = {};
Expand Down Expand Up @@ -62,12 +62,7 @@ define(
matched = matched.split(/\s+/).join(" ");
df.appendChild($("<em/>").attr({ "class": "rfc2119", title: matched }).text(matched)[0]);
// remember which ones were used
if (conf.respecRFC2119[matched]) {
conf.respecRFC2119[matched]++;
}
else {
conf.respecRFC2119[matched] = 1;
}
conf.respecRFC2119[matched] = true;
}
// BIBREF
else if (/^\[\[/.test(matched)) {
Expand Down