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

add ‘url’ parameter to type constructors to improve error messages #105

Merged
merged 1 commit into from
Dec 1, 2016

Conversation

davidchambers
Copy link
Member

//    inc :: FiniteNumber -> FiniteNumber
const inc = def('inc', {}, [$.FiniteNumber, $.FiniteNumber], x => x + 1);

inc(Infinity);
// ! TypeError: Invalid value
//
//   inc :: FiniteNumber -> FiniteNumber
//          ^^^^^^^^^^^^
//               1
//
//   1)  Infinity :: Number
//
//   The value at position 1 is not a member of ‘FiniteNumber’.
//
//   See https://github.com/sanctuary-js/sanctuary-def/tree/v0.7.0#FiniteNumber for information about the sanctuary-def/FiniteNumber type.

The link to the type's documentation strikes me as useful. What do you think? It makes defining types a bit less convenient, but provides a benefit to those who use functions defined via sanctuary-def.

Using version-specific URLs is a good idea as a type may be deleted or its meaning may change.

'The value at position 1 is not a member of ' +
showTypeQuoted(resolvePropPath(typeInfo.types[index], propPath)) + '.\n'
'The value at position 1 is not a member of ' + s + '.\n' +
(t.url &&
Copy link
Member Author

Choose a reason for hiding this comment

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

This means one can simply use '' as the url of an undocumented type.

@@ -380,17 +382,28 @@
};
}

function typeX(name, test, _1, _2) {
var version = '0.7.0'; // updated programmatically
Copy link
Member

Choose a reason for hiding this comment

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

can we get it from package.json

Copy link
Member Author

Choose a reason for hiding this comment

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

Not in the browser, unfortunately.

var version = '0.7.0'; // updated programmatically
var url = 'https://github.com/sanctuary-js/sanctuary-def/tree/v' +
version + '#' + stripNamespace(name);
switch (arguments.length) {
Copy link
Member

Choose a reason for hiding this comment

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

What about something like this? (this names are not perfect)

$.FiniteNumber = withURL('sanctuary-def/FiniteNumber', (name, url) => NullaryType(
  name, url,
  function(x) { return $.ValidNumber._test(x) && isFinite(x); }
));
// or 
$.FiniteNumber = typeWithURL(
  'sanctuary-def/FiniteNumber',
  NullaryType,
  function(x) { return $.ValidNumber._test(x) && isFinite(x); }
);

Whis way we would not need this switch statement. And it will eliminate need to update it if we add some other *Type with more arguments. Also it will still be clear which type a value is (user will scan the definition and see NullaryType or whatever immediately instead of counting number of arguments.

$.Pair = typeWithURL(
  'sanctuary-def/Pair',
  BinaryType,
  function(x) { return $$typeEq('Array')(x) && x.length === 2; },
  function(pair) { return [pair[0]]; },
  function(pair) { return [pair[1]]; }
)
// vs
$.Pair = typeX(
  'sanctuary-def/Pair',
  function(x) { return $$typeEq('Array')(x) && x.length === 2; },
  function(pair) { return [pair[0]]; },
  function(pair) { return [pair[1]]; }
)

Copy link
Member Author

Choose a reason for hiding this comment

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

Great suggestion, Irakli! I was not at all happy with typeX. I've updated the pull request. :)

@@ -1379,15 +1403,15 @@
//. // Rank :: Type
//. const Rank = $.NullaryType(
//. 'my-package/Rank',
//. x => typeof x === 'string' && /^([A23456789JQK]|10)$/.test(x),
//. 'A'
Copy link
Member

Choose a reason for hiding this comment

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

don't know what this 'A' and \u2660 are, but is it intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

'\u2660' is '♠'. Neither that nor 'A' belong where they are. I'm not sure how they came to be there, but we should remove them now that we're aware of the errors.

@safareli
Copy link
Member

safareli commented Dec 1, 2016

👍

@davidchambers davidchambers force-pushed the dc-type-urls branch 2 times, most recently from 00d1603 to da12ae3 Compare December 1, 2016 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants