Ticket #860 (closed feature: fixed)

Opened 6 years ago

Last modified 6 years ago

Cleanup getArgs(), Make Parse Comma-Separated Values

Reported by: euzuro Owned by:
Priority: critical Milestone: 2.5 Release
Component: Util Version: 2.4
Keywords: Cc:
State:

Description

the getArgs() function:

  • exposes/overrides global variables
  • has multiple return statements
  • does not correctly process values of type "-180,-90,90,180" (should parse into array)

Attachments

getArgs.patch Download (2.8 KB) - added by euzuro 6 years ago.
remove global vars (keyValue and pairs), remove return statement in middle of function, make value parsing into arrays when necessary
getArgs.2.patch Download (4.9 KB) - added by euzuro 6 years ago.
cleaned up the logic a little bit and added some nd and inline comments. also, renamed function to getParameters() which I think is clearer. We maintain getArgs() but mark as deprecated.
getArgs.3.patch Download (6.2 KB) - added by euzuro 6 years ago.
following in style, we'll throw a console warning on use of now deprecated getArgs(). Also add new test to make sure getArgs is working ok.
getArgs.4.patch Download (5.5 KB) - added by tschaub 6 years ago.
remove use of getArgs in the library

Change History

Changed 6 years ago by euzuro

remove global vars (keyValue and pairs), remove return statement in middle of function, make value parsing into arrays when necessary

Changed 6 years ago by euzuro

  • keywords review added

tests pass ff & ie6. faz favor, o review

Changed 6 years ago by euzuro

new patch even prettier than the first. all tests pass ff & ie6

Changed 6 years ago by euzuro

cleaned up the logic a little bit and added some nd and inline comments. also, renamed function to getParameters() which I think is clearer. We maintain getArgs() but mark as deprecated.

Changed 6 years ago by crschmidt

No feedback on list, seems like it's good to commit.

Changed 6 years ago by euzuro

following in style, we'll throw a console warning on use of now deprecated getArgs(). Also add new test to make sure getArgs is working ok.

Changed 6 years ago by euzuro

updated patch. please review i will commit

Changed 6 years ago by crschmidt

  • keywords commit added; review removed

Looks good -- go ahead to commit

Changed 6 years ago by euzuro

  • keywords commit removed
  • status changed from new to closed
  • resolution set to fixed

(In [4052]) rename and deprecate getArgs() function in favor of getParameters(), make it such that it parses comma-separated values from key/value pairs into Arrays (since they are encoded that way). (Closes #860)

Changed 6 years ago by tschaub

  • status changed from closed to reopened
  • resolution fixed deleted

Let's get rid of getArgs in the library if we're going to be issuing warnings about it.

Changed 6 years ago by tschaub

remove use of getArgs in the library

Changed 6 years ago by tschaub

  • keywords review added

All tests pass. Please review.

Changed 6 years ago by crschmidt

  • keywords commit added; review removed

Good point. Sorry I missed that.

Changed 6 years ago by tschaub

  • keywords commit removed
  • status changed from reopened to closed
  • resolution set to fixed

(In [4080]) removing use of getArgs from the library (closes #860).

Changed 6 years ago by euzuro

excellent call. thanks 4or being on the lookout, tim :-)

Note: See TracTickets for help on using tickets.