Page MenuHome

Fix T82407: Negative number input gives syntax error for velocities and accelerations
ClosedPublic

Authored by Philipp Oeser (lichtwerk) on Nov 5 2020, 11:33 AM.

Details

Summary

Caused by rB45dbc38a8b15.

Above commit would place parentheses surrounding a block until the next
operator was found.
For velocities and accelerations though, the '/' in 'm/s' or 'ft/s'
should not be considered an operator.

Diff Detail

Repository
rB Blender
Branch
T82407 (branched from master)
Build Status
Buildable 11135
Build 11135: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Nov 5 2020, 11:33 AM
Hans Goudey (HooglyBoogly) requested changes to this revision.Nov 5 2020, 4:56 PM

The patch itself makes sense, but I wonder if it would make more sense to check for any letter a through z instead.

If not, we should use the ELEM macro here, that makes this check look much better.

I also noticed that scientific_notation variable isn't doing anything, so I can clean that up later.

This revision now requires changes to proceed.Nov 5 2020, 4:56 PM
  • use ELEM macro
  • be more precise and only consider '/'

The patch itself makes sense, but I wonder if it would make more sense to check for any letter a through z instead.

why not be precise? what other cases do you have in mind?

If not, we should use the ELEM macro here, that makes this check look much better.

done

I also noticed that scientific_notation variable isn't doing anything, so I can clean that up later.

please do

why not be precise? what other cases do you have in mind?

Mostly because the characters we check for here depend on the strings in each bUnitDef, so we're introducing a dependency that won't be obviously visible in the future. But it's just a trade-off I suppose, because this is more explicit, and I'm fine with this.

This revision is now accepted and ready to land.Nov 5 2020, 5:32 PM