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

Port environments.js to @flow. #862

Merged
merged 2 commits into from Sep 10, 2017
Merged
Show file tree
Hide file tree
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
125 changes: 92 additions & 33 deletions src/environments.js
@@ -1,18 +1,96 @@
/* eslint no-constant-condition:0 */
// @flow
import ParseNode from "./ParseNode";
import ParseError from "./ParseError";

import type Parser from "./Parser";
import type {ArgType, Mode, StyleStr} from "./types";

/**
* The context contains the following properties:
* - mode: current parsing mode.
* - envName: the name of the environment, one of the listed names.
* - parser: the parser object.
* - positions: the positions associated with these arguments from args.
*/
type EnvContext = {
mode: Mode,
envName: string,
parser: Parser,
positions: number[],
};

/**
* List of ParseNodes followed by an array of positions.
* Returned by `Parser`'s `parseArguments`.
*/
type ParseResult = (ParseNode | number[])[] | ParseNode;
Copy link
Member

Choose a reason for hiding this comment

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

When do we return a result with type number[][]?

Copy link
Collaborator Author

@marcianx marcianx Sep 9, 2017

Choose a reason for hiding this comment

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

Parser's parseArgument always returns the list of positions as the last element.
So it returns something of the format [ParseNode, ParseNode, ..., ParseNode, number[]]. So this happens any time you see a defineFunction with code like

    res = parseArray(context.parser, res, "display");
    return res;

We need to refactor this at some point, but I'd rather do it right before or during ParseNode being ported. It would help to have maximal support from type-safety while doing this since ParseResult is simply stored here and used various places (including possible buildHtml, but I haven't read that part of the code base yet).

Copy link
Member

Choose a reason for hiding this comment

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

I see, it's documented in the comment above.

We need to refactor this at some point, but I'd rather do it right before or during ParseNode being ported.

Sounds good.


/**
* The handler function receives two arguments
* - context: information and references provided by the parser
* - args: an array of arguments passed to \begin{name}
*/
type EnvHandler = (context: EnvContext, args: ParseNode[]) => ParseResult;

/**
* - numArgs: (default 0) The number of arguments after the \begin{name} function.
* - argTypes: (optional) Just like for a function
* - allowedInText: (default false) Whether or not the environment is allowed
* inside text mode (not enforced yet).
* - numOptionalArgs: (default 0) Just like for a function
*/
type EnvProps = {
numArgs?: number,
argTypes?: ArgType[],
allowedInText?: boolean,
numOptionalArgs?: number,
};

type EnvData = {
numArgs: number,
argTypes?: ArgType[],
greediness: number,
allowedInText: boolean,
numOptionalArgs: number,
handler: EnvHandler,
};
Copy link
Member

Choose a reason for hiding this comment

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

This looks suspiciously similar to how functions are typed. We might be able to dedupe in future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI: Looking at defineFunction.js I don't currently see any benefit in coalescing.

  1. We don't really need any subtype relationship.
  2. For object/struct types, there's not really an extends type behavior AFAICS (please correct me if I'm wrong), so we would need to repeat the fields anyway when defining it.

const environments: {[string]: EnvData} = {};
export default environments;

// Data stored in the ParseNode associated with the environment.
type AlignSpec = { type: "separator", separator: string } | {
type: "align",
align: string,
pregap?: number,
postgap?: number,
};
type ArrayEnvNodeData = {
type: "array",
hskipBeforeAndAfter?: boolean,
arraystretch?: number,
addJot?: boolean,
cols?: AlignSpec[],
// These fields are always set, but not on struct construction
// initialization.
body?: ParseNode[][], // List of rows in the (2D) array.
rowGaps?: number[],
};

/**
* Parse the body of the environment, with rows delimited by \\ and
* columns delimited by &, and create a nested list in row-major order
* with one group per cell. If given an optional argument style
* ("text", "display", etc.), then each cell is cast into that style.
*/
function parseArray(parser, result, style) {
function parseArray(
parser: Parser,
result: ArrayEnvNodeData,
style: StyleStr,
) {
let row = [];
const body = [row];
const rowGaps = [];
while (true) {
for (;;) {
Copy link
Member

Choose a reason for hiding this comment

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

Why the change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, that was just to get rid of the eslint suppression at the top (personally a fan of maximal linting). However, that's not my favorite lint anyway; and I can revert this if you like.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. This is fine. We can re-evaluate lint rules separately.

let cell = parser.parseExpression(false, null);
cell = new ParseNode("ordgroup", cell, parser.mode);
if (style) {
Expand Down Expand Up @@ -42,36 +120,17 @@ function parseArray(parser, result, style) {
return new ParseNode(result.type, result, parser.mode);
}


/*
* An environment definition is very similar to a function definition:
* it is declared with a name or a list of names, a set of properties
* and a handler containing the actual implementation.
*
* The properties include:
* - numArgs: The number of arguments after the \begin{name} function.
* - argTypes: (optional) Just like for a function
* - allowedInText: (optional) Whether or not the environment is allowed inside
* text mode (default false) (not enforced yet)
* - numOptionalArgs: (optional) Just like for a function
* A bare number instead of that object indicates the numArgs value.
*
* The handler function will receive two arguments
* - context: information and references provided by the parser
* - args: an array of arguments passed to \begin{name}
* The context contains the following properties:
* - envName: the name of the environment, one of the listed names.
* - parser: the parser object
* - lexer: the lexer object
* - positions: the positions associated with these arguments from args.
* The handler must return a ParseResult.
* it is declared with a list of names, a set of properties and a handler
* containing the actual implementation.
*/
function defineEnvironment(names, props, handler) {
if (typeof names === "string") {
names = [names];
}
if (typeof props === "number") {
props = { numArgs: props };
}
function defineEnvironment(
names: string[],
props: EnvProps,
handler: EnvHandler,
) {
// Set default values of environments
const data = {
numArgs: props.numArgs || 0,
Expand All @@ -82,7 +141,7 @@ function defineEnvironment(names, props, handler) {
handler: handler,
};
for (let i = 0; i < names.length; ++i) {
module.exports[names[i]] = data;
environments[names[i]] = data;
}
}

Expand Down Expand Up @@ -207,7 +266,7 @@ defineEnvironment([
// except it operates within math mode.
// Note that we assume \nomallineskiplimit to be zero,
// so that \strut@ is the same as \strut.
defineEnvironment("aligned", {
defineEnvironment(["aligned"], {
}, function(context) {
let res = {
type: "array",
Expand Down Expand Up @@ -252,7 +311,7 @@ defineEnvironment("aligned", {
// A gathered environment is like an array environment with one centered
// column, but where rows are considered lines so get \jot line spacing
// and contents are set in \displaystyle.
defineEnvironment("gathered", {
defineEnvironment(["gathered"], {
}, function(context) {
let res = {
type: "array",
Expand Down
3 changes: 3 additions & 0 deletions src/types.js
Expand Up @@ -16,3 +16,6 @@ export type Mode = "math" | "text";
// first argument is special and the second
// argument is parsed normally)
export type ArgType = "color" | "size" | "original";

// LaTeX display style.
export type StyleStr = "text" | "display";