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

Expressions starting from `_` are treated as placeholders in `match` cases #1025

Closed
pepkin88 opened this Issue Mar 26, 2018 · 7 comments

Comments

Projects
None yet
3 participants
@pepkin88
Contributor

pepkin88 commented Mar 26, 2018

Example:

match a
| _.isString => 1
| (_.isString) => 1

Result:

var ref$;
switch (ref$ = [a], false) {
case !true:
  1;
  break;
case !_.isString(ref$[0]):
  1;
}

Expected result:

var ref$;
switch (ref$ = [a], false) {
case !_.isString(ref$[0]):
  1;
  break;
case !_.isString(ref$[0]):
  1;
}

The example shows, that wrapping the expression in parentheses is enough to bypass this bug.

@rhendric

This comment has been minimized.

Collaborator

rhendric commented Mar 26, 2018

Oh, match. The undocumented, largely untested, experimental feature that somehow people keep discovering anyway.

This sure does look like a bug, but I don't know whether this should be an error, your expected result, or something else. _ has special meaning inside match, which is complicated by the fact that _ is also used to create a partial-application function and as far as I know, every case of a match expression should be interpreted as a function (unless it isn't, like with constants).

I don't really intend to do anything to improve the state of match, unless someone feels like fully and properly specifying its behavior. The good news is, as an experimental undocumented feature, I also wouldn't feel even a tiny bit bad about ripping it out and replacing it with something else. You were all warned. 🙂

@pepkin88

This comment has been minimized.

Contributor

pepkin88 commented Mar 26, 2018

OK, fair enough.

I drew an analogy to the partial application or switch's default, where only a standalone underscore has a special meaning. In those cases using _.toString wouldn't trigger anything special.

I might fix it later. For now, let this issue be a walk-around suggestion.

@pepkin88

This comment has been minimized.

Contributor

pepkin88 commented Apr 2, 2018

Related issue with a plausible use case scenario:

This:

(_.isPlainObject || _.isArray) value

compiles to:

true || true;

instead of:

_.isPlainObject(value) || _.isArray(value);

Although I think this feature is a real gotcha and makes sense only in the context of match, it actually is documented.

@rhendric

This comment has been minimized.

Collaborator

rhendric commented Apr 2, 2018

Ha, okay, you got me! This should be fixed.

@rhendric rhendric added the bug label Apr 2, 2018

@pepkin88

This comment has been minimized.

Contributor

pepkin88 commented Apr 2, 2018

This looks like an easy one I could tackle, but only from next week.

@rhendric

This comment has been minimized.

Collaborator

rhendric commented Apr 2, 2018

I'll hold it for you then.

@anant-svc

This comment has been minimized.

anant-svc commented Jun 11, 2018

Hi, tried to get expected result @pepkin88 needed using below:

u = _
match a
| u.isString => 1
| (u.isString) => 1

Result:

var u, ref$;
u = _;
switch (ref$ = [a], false) {
case !u.isString(ref$[0]):
  1;
  break;
case !u.isString(ref$[0]):
  1;
}

pepkin88 added a commit to pepkin88/LiveScript that referenced this issue Jul 29, 2018

@pepkin88 pepkin88 referenced this issue Jul 29, 2018

Merged

Fix for #1025 #1059

pepkin88 added a commit to pepkin88/LiveScript that referenced this issue Jul 29, 2018

pepkin88 added a commit to pepkin88/LiveScript that referenced this issue Jul 29, 2018

pepkin88 added a commit to pepkin88/LiveScript that referenced this issue Jul 29, 2018

fix gkz#1025
actual fix

@rhendric rhendric closed this in 7b91a13 Jul 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment