cubing/alg.js

The functionality of `alg.js` is now part of `cubing.js`. Please see: https://js.cubing.net/cubing/alg/

alg.js's Issues

Define `toString` directly on Algorithm subclasses?

Pros:

• Nice, concise formatting by default! (node actually truncates nested values)
• Can still convert to JSON, so inspectability is not lost.

Cons:

• Javascript does magic type conversion, and this could cause unexpected issues. For all I know 1 + [alg representing 2R] could turn into 3.
• Viewing the printed alg instead of the structure might hide unexpected discrepancies.

A nested sequence containing a single unit is redundant

The following prints false:

var original = new Alg.Group(
  new Alg.Sequence([
    new Alg.BlockMove("R", 1)
  ]),
  1
)
var parsed = parse(algToString(original));
console.log(structureEquals(original, parsed));

Ideally, I'd like there to be only a single representation of this alg.
One possibility is to have a nested kind of sequence that must have at least two units in it. The parser actually uses this requirement to avoid unnecessary nesting (which is why parsed is different):

alg.js/src/jison_parser/index.jison

Line 147 in 716c91d

: REPEATED_UNIT OPTIONAL_WHITESPACE REPEATED_UNIT

Ideas:

1. Create a different kind of sequence for a nested alg that requires at least two members.

• Pro: The top-level alg class could also handle the empty alg (#5).
• Con: Nesting certain (or all) top-level algs inside another alg would not be possible without changing the top-level representation into an equivalent nested representation.

2. Define every Algorithm with nested algs to have its own lists, e.g. a commutator has {A: Unit[], B: Unit[]}. This requires making sure to call the Sequence processing code for each of the components in every relevant Algorithm in every traversal, possibly leading to subtle bugs.
3. Don't allow anything except a Sequence to contain a Unit, and require a Sequence layer.

• This results in larger data structures, and an extra level of access in some cases. However, code might be more consistent, because looking inside a nested alg always requires going through a Sequence the same way.

See https://gist.github.com/lgarron/f33d88a571f4bc9536801a8a7b035a91 for more thoughts on this.

Try out immutable data structures for Sequence

https://facebook.github.io/immutable-js/

But not without benchmarks.

Reintroduce a `release` branch?

And include dist on the release branch for e.g. source downloads with the built file.

This is one way to solve #15.

Can't keep class names using `uglifyjs-webpack-plugin` with "mode": "production"

As soon as I set module.exports.mode = "production", class names are mangled.

Some references:
https://webpack.js.org/plugins/uglifyjs-webpack-plugin/
https://github.com/webpack-contrib/uglifyjs-webpack-plugin

Absolutely nothing works to keep the class names, including this overkill config:

plugins: [
  new UglifyJSPlugin({
    sourceMap: true,
    uglifyOptions: {
      keep_classnames: true,
      "mangle": {
        keep_classnames: true
      },
      "compress": {
        keep_classnames: true
      }
    }
  })
],

Or this one:

optimization: {
  minimizer: [
    new UglifyJSPlugin({
      sourceMap: true,
      uglifyOptions: {
        keep_classnames: true,
        "mangle": {
          keep_classnames: true
        },
        "compress": {
          keep_classnames: true
        }
      }
    })
  ]
},

I know that the config is being parsed properly, because I get errors if I place keep_classnames directly in the UglifyJSPlugin config object or change keep_classnames into a typo anywhere.

Accept more characters as "PRIME" (apostrophes, accents, etc)

People from different cultures use different kinds of apostrophe or accents to denote algorithms and solve reconstructions.

I would suggest replacing the line 22 of alg_jison.jison to accept at least these 4 most common ones:

['’`´]             return "PRIME"

Test `TraversalDownUp` and `TraversalUp` again.

Empty alg

The following throws A sequence cannot be empty.:

var s = alg.parse("L L'")
alg.coalesceBaseMoves(s)

I'm currently leaning towards a special, non-nestable EmptyAlgorithm extends Algorithm class to represent the empty alg, if anything.

Restore chaining?

e.g.

parse("[F: [R, U]]').invert().expand().structureEquals(parse("F U R U' R' F'"))

Also see #7 (a special case for toString() and #9 (providing convenience for when you just want to manipulate strings.

This is also only easy to provide for built-ins, although Javascript technically supports modifying class prototypes downstream from the original definition.

`expand(parse("(R U)2"))` fails.

Using `jison-loader` to import the `jison` file?

https://github.com/joelburget/jison-loader

`yarn publish` ignores `dist` folder

Filing a bug for posterity.

yarn publish follows https://docs.npmjs.com/files/package.json#files and ignores the dist folder if it's in our `.gitignore

The solution was to do something like https://github.com/bcherny/undux and include yet another dotfile (.npmignore).

Make it easy to download `alg.js` directly.

`AlgPart` constructors require `new`, `____SiGNMove` helpers cannot be called with `new`

Example:

new Sequence([BareSiGNMove("R")])

It's probably confusing to beginners to know when to use new. TypeScript and ES6 require new for classes, and I don't want to create a helper for every class. So the main workaround I see here is to make SiGNMove an abstract class, and SiGNMove helpers into actual subclasses of SiGNMove.

Shorted `nestedUnits` and `nestedSequence`?

Use Chai's built-in frozen object test.

http://www.chaijs.com/api/bdd/#method_frozen

Allow manipulating strings directly?

This is less performant, but super convenient:

> expand("(R U)2")
"R U R U"

Can easily be done by separating the Traversal[Down]Up.traverse() entry point from a recursive entry point, and translating the input from/to a string if it was given as a string.

The main problem I see is that the performance issue might be masked, and users might perform a lot of conversions back and forth:

structureEquals(invert(expand("(R U)2")), expand(invert("(R U)2")))

Bundle definitions into a single `index.d.ts` file.

It seems there is no common solution for this yet, so we're stuck with a whole bunch of d.ts files in ./dist for now.

Testing

• 2L L' should not simplify

Automatically convert string locations to indices.

We're not dealing with "source code", so I imagine direct indices are more useful.

Hopefully, this won't take extra processing. If possible, we should:

• Modify/set a parameter in jison to output string indices.
• If this isn't possible, defer calculation or do something else clever.

Slice moves, wide block moves, and rotations