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
Implement strict mode (replacing unicodeTextInMathMode) #1278
Changes from 7 commits
1b6db8e
a1f292a
f8da0cc
4af450a
35d9876
f8400f6
d79f653
c1694f0
222f6eb
8b8d794
905bfd8
890288d
b10418f
22895dc
ca212c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,28 @@ | ||
// @flow | ||
/* eslint no-console:0 */ | ||
/** | ||
* This is a module for storing settings passed into KaTeX. It correctly handles | ||
* default settings. | ||
*/ | ||
|
||
import utils from "./utils"; | ||
import ParseError from "./ParseError.js"; | ||
import ParseNode from "./ParseNode"; | ||
import {Token} from "./Token"; | ||
|
||
import type { MacroMap } from "./macros"; | ||
|
||
export type StrictFunction = | ||
(errorCode: string, errorMsg: string, token?: Token | ParseNode) => | ||
?(boolean | string); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exposing this level of customizability to users seems like overkill. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This level of customizability lets users allow some nonstrict behavior and not others. @ylemkimon at least seemed to think this would be useful, and it's not exactly hard to support. Thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another point for allowing a function is that it enables custom warning functionality, if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's nice to have customizability as long as we have simpler options to set them. We may add lots of features to the non-strict mode, and |
||
|
||
export type SettingsOptions = { | ||
displayMode?: boolean; | ||
throwOnError?: boolean; | ||
errorColor?: string; | ||
macros?: MacroMap; | ||
colorIsTextColor?: boolean; | ||
unicodeTextInMathMode?: boolean; | ||
strict?: boolean | "ignore" | "warn" | "error" | StrictFunction; | ||
maxSize?: number; | ||
}; | ||
|
||
|
@@ -34,7 +42,7 @@ class Settings { | |
errorColor: string; | ||
macros: MacroMap; | ||
colorIsTextColor: boolean; | ||
unicodeTextInMathMode: boolean; | ||
strict: boolean | "ignore" | "warn" | "error" | StrictFunction; | ||
maxSize: number; | ||
|
||
constructor(options: SettingsOptions) { | ||
|
@@ -45,10 +53,37 @@ class Settings { | |
this.errorColor = utils.deflt(options.errorColor, "#cc0000"); | ||
this.macros = options.macros || {}; | ||
this.colorIsTextColor = utils.deflt(options.colorIsTextColor, false); | ||
this.unicodeTextInMathMode = | ||
utils.deflt(options.unicodeTextInMathMode, false); | ||
this.strict = utils.deflt(options.strict, false); | ||
this.maxSize = Math.max(0, utils.deflt(options.maxSize, Infinity)); | ||
} | ||
|
||
/** | ||
* Report nonstrict (non-LaTeX-compatible) input. | ||
* Can safely not be called if `this.strict` is false in JavaScript. | ||
*/ | ||
nonstrict(errorCode: string, errorMsg: string, token?: Token | ParseNode) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this. Maybe rename There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was matching |
||
let strict = this.strict; | ||
if (typeof strict === "function") { | ||
// Allow return value of strict function to be boolean or string | ||
// (or null/undefined, meaning no further processing). | ||
strict = strict(errorCode, errorMsg, token); | ||
} | ||
if (!strict || strict === "ignore") { | ||
return; | ||
} else if (strict === true || strict === "error") { | ||
throw new ParseError( | ||
"LaTeX-incompatible input and strict mode is set to 'error': " + | ||
`${errorMsg} [${errorCode}]`, token); | ||
} else if (strict === "warn") { | ||
typeof console !== "undefined" && console.warn( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't think of such an environment, but presumably they might exist (or the global console could be overwritten?). In any case, I'm following existing code from the KaTeX codebase, in particular: typeof console !== "undefined" && console.warn(
"No character metrics for '" + value + "' in style '" +
fontName + "'"); It might be good to define a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh wow, I didn't realize we were doing that elsewhere. |
||
"LaTeX-incompatible input and strict mode is set to 'warn': " + | ||
`${errorMsg} [${errorCode}]`); | ||
} else { // won't happen in type-safe code | ||
typeof console !== "undefined" && console.warn( | ||
"LaTeX-incompatible input and strict mode is set to " + | ||
`unrecognized '${strict}': ${errorMsg} [${errorCode}]`); | ||
} | ||
} | ||
} | ||
|
||
export default Settings; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,17 @@ function init() { | |
options.displayMode = false; | ||
} | ||
|
||
// Use `strict=warn` for warning strict mode or `strict=error` | ||
// (or `=1`/`=t`/`=true`/`=y`/`=yes`) | ||
// to turn off displayMode (which is on by default). | ||
if (query.strict) { | ||
if (query.strict.match(/^(1|t|y|e)/)) { | ||
options.strict = "error"; | ||
} if (query.strict && query.strict.match(/^(w)/)) { | ||
options.strict = "warn"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the simplicity of this. |
||
} | ||
} | ||
|
||
// The `before` or `pre` search parameter puts normal text before the math. | ||
// The `after` or `post` search parameter puts normal text after the math. | ||
// Example use: testing baseline alignment. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about allowing all unicodes(non-ASCII) on non-strict mode? (#1217,
allowAllSymbols
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering about that, but planning to deal with it when we return to discussing #1217.
allowAllSymbols
feels like more of a hack, not to be encouraged -- the rendering will probably be poor in general, and it's really a sign that we need to add symbols to the fonts. So I was thinking of keeping that a separate option... but I'm open to suggestions. (And maybe it is worth discussing here, in case we want to tweak thestrict
functionality to deal with it...)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's where the customizability should come in. Normal users would intend used symbols to be shown, but advanced users or who worries about the layout would want to know unsupported symbol has been used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I agree. I think this is more motivation to set the default
strict
value to"warn"
, so at least people get a sense of why the rendering is funny in these cases.