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

Negating variable value causes variable to not be recognized #980

Open
kbaltrinic opened this issue Mar 11, 2013 · 26 comments
Open

Negating variable value causes variable to not be recognized #980

kbaltrinic opened this issue Mar 11, 2013 · 26 comments

Comments

@kbaltrinic
Copy link

Found in 0.32.0. An example says it all:

my-val = 20px

#my-element
  top -my-val

renders as:

#my-element{
  top: -my-val;
}

workaround, separating the operator from the variable works:

#my-element
  top - my-val

renders as expected:

#my-element{
  top: -20px;
}

The workaround is more complicated when dealing with a list of values:

#my-element
  background-position 5px - my-val

renders as:

#my-element{
   background-position: -15px;
}

Oops, unintended math! Instead the following slightly uglier workaround works:

#my-element
  background-position 0 (- my-val)

In the end, this looks like an issue with preceding dashes being interpreted as part of the variable name (just a guess?) I did not see anything in the stylus docs about legal variable names but they probably shouldn't be allowed to start with dashes.

@vendethiel
Copy link
Contributor

this looks like an issue with preceding dashes being interpreted as part of the variable name (just a guess?)

indeed.

@kizu
Copy link
Member

kizu commented Jun 30, 2013

I wonder it's a bit late to disallow usage of dashes, but when we'd get to the next major release, like 1.0, we could do this (along with a lot of other things that could potentially break everything for someone).

Now we should update the docs mentioning this issue I think.

@vendethiel
Copy link
Contributor

we consider everything to be an identifier, don't we ? If we disallow it, -webkit-stuff wouldn't work anymore, would it?

@kizu
Copy link
Member

kizu commented Jun 30, 2013

Ah, true, my bad. Then, it's just a matter of mentioning this in Docs.

@kbaltrinic
Copy link
Author

Beyond mentioning it in the docs, could we be smarter about this? If we see something like -my-val and -my-val is not defined but my-val is, can we emit an appropriate warning that points out to people the error of their ways.

@kbaltrinic
Copy link
Author

It may also be possible to come up with an alternative syntax for negation that would be unambiguous but better than (- my-val). Not sure if there really is a better syntax but it my be worth brainstorming over. This would be in addition to the warning because you still need to account for the people who are going to try to do the the obvious but wrong thing.

@kizu
Copy link
Member

kizu commented Jul 1, 2013

I remembered what I thought when I said we could make it in 1.0.

One of the most ambiguous thing in Stylus that gives a lot of problems are variables — one of the ideas for 1.0 was to leave only the $foo syntax, it would stand out from the idents, so -$foo could become the negation without any problems.

Also, I think we could make it work this way in the current versions of Stylus, because there are no idents containing -$, but that would make the two notations of variables to be different.

@kbaltrinic
Copy link
Author

I like the idea of requiring the $. You might want to make it a compiler switch option in the next version; sort of making the $-less version still supported but deprecated so to speak and then maybe fully drop support later.

@vendethiel
Copy link
Contributor

I approve for $.

@jasonkuhrt
Copy link
Contributor

Would $ affect function and mixin names? While I am okay with $var and would even welcome it we should guard the transparent mixin/function ability of stylus.

@vendethiel
Copy link
Contributor

Only for variables imho.

@kizu
Copy link
Member

kizu commented Jul 1, 2013

Yes, it should be only for variables.

@ameliemaia
Copy link

Another alternative to achieve negative values is -(var)

I would definitely opt for stylus to recognise dashes as the minus operator when used on a number variable. Although I'm not too sure how difficult this would be to implement to the parser ?

@samholmes
Copy link

What's the progress on this? The only solutions right now is to do:

property (-(var))

or

property var * -1

Both are ugly.

Why not just allow for "-(var)"? Or is there a negative() function?

@vendethiel
Copy link
Contributor

-(var) works here

@samholmes
Copy link

-(var) cause this error for me:

ReferenceError: /Users/holmes/Code/xxxxxx/app/views/layouts/og-default.jade:6
    4| meta(property="og:title", content=ogTitle)
    5| meta(property="og:type", content=ogType)
  > 6| meta(property="og:url", content=ogUrl)
    7| meta(property="og:image", content=ogImage + "?" + ogRandomNum)
    8| if ogImageSecure
    9|  meta(property="og:image:secure_url", content=ogImageSecure + "?" + ogRandomNum)

ogUrl is not defined
    at eval (eval at <anonymous> (/Users/holmes/Code/xxxxxx/app/node_modules/jade/lib/jade.js:176:8), <anonymous>:86:65)
    at /Users/holmes/Code/xxxxxx/app/node_modules/jade/lib/jade.js:181:12
    at Object.exports.render (/Users/holmes/Code/xxxxxx/app/node_modules/jade/lib/jade.js:216:14)
    at View.exports.renderFile [as engine] (/Users/holmes/Code/xxxxxx/app/node_modules/jade/lib/jade.js:243:13)
    at View.render (/Users/holmes/Code/xxxxxx/app/node_modules/express/lib/view.js:75:8)
    at Function.app.render (/Users/holmes/Code/xxxxxx/app/node_modules/express/lib/application.js:504:10)
    at ServerResponse.res.render (/Users/holmes/Code/xxxxxx/app/node_modules/express/lib/response.js:753:7)
    at errorLogHandler (/Users/holmes/Code/xxxxxx/app/controllers/errors.js:44:8)
    at queryHandler (/Users/holmes/Code/xxxxxx/app/models/errors.js:25:3)
    at Query.<anonymous> (/Users/holmes/Code/xxxxxx/app/node_modules/mysql/lib/client.js:106:11)
    at Query.EventEmitter.emit (events.js:95:17)
    at Query._handlePacket (/Users/holmes/Code/xxxxxx/app/node_modules/mysql/lib/query.js:29:12)
    at Client._handlePacket (/Users/holmes/Code/xxxxxx/app/node_modules/mysql/lib/client.js:319:14)
    at Parser.EventEmitter.emit (events.js:95:17)
    at emitPacket (/Users/holmes/Code/xxxxxx/app/node_modules/mysql/lib/parser.js:71:14)
    at Parser.write (/Users/holmes/Code/xxxxxx/app/node_modules/mysql/lib/parser.js:576:7)
    at Socket.EventEmitter.emit (events.js:95:17)
    at Socket.<anonymous> (_stream_readable.js:736:14)
    at Socket.EventEmitter.emit (events.js:92:17)
    at emitReadable_ (_stream_readable.js:408:10)
    at emitReadable (_stream_readable.js:404:5)
    at readableAddChunk (_stream_readable.js:165:9)
    at Socket.Readable.push (_stream_readable.js:127:10)
    at TCP.onread (net.js:526:21)

@vendethiel
Copy link
Contributor

that's jade related. not the correct error message

@fetis
Copy link

fetis commented Apr 15, 2014

I have this issue in the latest version (0.43.1)

My code

var = 50px
body {
  width var;
  margin-left -var
}
body {
  width: 50px;
  margin-left: -var;
}

@Panya
Copy link
Member

Panya commented Apr 15, 2014

- is a part of the ident -var. You should use brackets to get negative value. For example:

var = 50px
body {
  width var;
  margin-left -(var)
}

@fetis
Copy link

fetis commented Apr 30, 2014

@Panya actually, this method also buggy. I got next problem

margin gallery_gap 0 gallery_gap -(gallery_gap)

produces me

margin: 45px 0 0px;

@Panya
Copy link
Member

Panya commented Apr 30, 2014

@fetis this is an edge case, you can write:

margin gallery_gap 0 gallery_gap (-(gallery_gap))

@fetis
Copy link

fetis commented Apr 30, 2014

what about to exclude '-' from variables identifiers?

@Panya
Copy link
Member

Panya commented Apr 30, 2014

In 1.0, sure.

@fetis
Copy link

fetis commented Apr 30, 2014

but this'll be not very soon, will it?

@Panya
Copy link
Member

Panya commented Apr 30, 2014

It's not matter of time, this will be a breaking change, and following semver it should be in the major release. But if you want the estimates, we'll release 1.0 in this year :)

@maranomynet
Copy link
Contributor

Requiring a space when negating variables (e.g. - var) is quite acceptable IMO.

Also since you can wrap the whole negation in parenthesis (i.e. padding: 0 (- var);) to isolate it from other space-delimited values this seems IMO like a very acceptable Stylus "eccentricity" that only needs to be clearly documented.

Much rather than introducing weird inconsistencies in how math with variables works, or unnecessary breaking changes.

@kizu kizu added this to the 1.0 milestone Sep 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants