-
Notifications
You must be signed in to change notification settings - Fork 106
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 display_list #738
Conversation
Used to have lots of annoying spaces at the end of a multiline list. I've gotten rid of those spaces so the end of lists are simply suffixed with ']'.
Pull Request Test Coverage Report for Build 3805
💛 - Coveralls |
45aa340
to
0b28b51
Compare
Played a bit. Looks really solid. Good job @thomastanck ! Will try to find a reviewer. |
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.
other than the infinite lists, let's have some tests of list of pairs, to make sure they display properly like
list(pair(1, 2), pair(3, 4));
src/utils/stringify.ts
Outdated
return ( | ||
typeof v.replPrefix !== 'undefined' && | ||
typeof v.replSuffix !== 'undefined' && | ||
typeof v.replArrayContents !== 'undefined' |
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.
since it needs to be a function
typeof v.replArrayContents !== 'undefined' | |
typeof v.replArrayContents === 'function' |
maybe can check that the prefix and suffix are strings instead too
2d3d60b
to
0dec3c2
Compare
Works by measuring a bunch of size vs runtime points, and doing linear regression on a log-log plot, using the resulting gradient as an estimate for the power of the runtime complexity.
replPrefix and replSuffix should be strings, replArrayContents should be a function.
0dec3c2
to
39e872b
Compare
Resolves #379, and also changes the standard
stringify
of lists slightly, see eac6949.The implementation does two passes over the list structure. It first performs a deep copy of the entire list structure, and at the same time identifies nodes which are list-like by constructing a ListObject for it. On the first pass, we keep all references as references to the original list structure for simplicity. In a second pass, it then replaces references to the old list structure, using set_head/tail to point these to the new list structure. We don't process the same pair more than once, so runtime complexity is linear in the number of pairs in the list structure.
The implementation assumes that the whole thing is a pure list structure, and there are no objects. To fix this for objects is not too tricky: we just have to add more special cases to the top of the
visit
function, to have thevisit
function recursively call object members, and also recreate objects during the second pass.This PR also exposes a new interface similar to
toReplString
, that we use to implement the pretty printing. Now, if an object contains the propertiesreplPrefix
,replSuffix
, andreplArrayContents
, it will assume the object is "array-like", and will stringify the contents and join them very similarly to how it handles pairs/arrays. This means formatting logic (e.g. multiline handling) is kept withinstringify
, and can be changed more easily if we need to.This raises the possibility of perhaps pretty printing runes created by
beside
, etc. as those are also "array-like".