sutoiku / formula.js

JavaScript implementation of most Microsoft Excel formula functions
Other
2.14k stars 293 forks source link

Fixing SEARCH function to return error.value when text is not found #32

Closed petermoresi closed 9 years ago

petermoresi commented 9 years ago

When I look at the MS documentation it says that:

If the value of find_text is not found, the #VALUE! error value is returned.

http://office.microsoft.com/en-001/excel-help/search-searchb-functions-HP010062577.aspx

A common trick is to use ISNUMBER(SEARCH("foo", "barbar")) which should return FALSE.

0x333333 commented 9 years ago

Hi @fgodino and @petermoresi, there is a small issue:

https://github.com/petermoresi/formula.js/blob/master/lib/text.js#L244

foundAt = within_text.toLowerCase().indexOf(find_text.toLowerCase(), position - 1);
return (foundAt === -1)?error.value:foundAt;

In Excel, the first index of an array is 1, It's different than common programming language who starts an array by 0.

By the way, it can be better if you could add some tests here: https://github.com/sutoiku/formula.js/blob/master/test/text.js#L159.

Thank you!

petermoresi commented 9 years ago

I see, I need to add 1 to the result and check for 0 instead of -1.

Great thought to add tests but I don't know how to run the test suite. Are there instructions?

I'll close this pull request and send another when it is corrected.

0x333333 commented 9 years ago

Thanks @petermoresi , Sorry for the lack of documents, it will be done soon.

For now you can run test like this:

make test

https://github.com/sutoiku/formula.js/blob/master/Makefile#L12

petermoresi commented 9 years ago

No worries. I took a look at the Makefile and figure out what to do.

For the latest pull request I ran "make test" before submitting.

On Wed, Dec 3, 2014 at 8:06 PM, Zhipeng JIANG notifications@github.com wrote:

Thanks @petermoresi https://github.com/petermoresi , Sorry for the lack of documents, it will be done soon.

For now you can run test like this:

make test

https://github.com/sutoiku/formula.js/blob/master/Makefile#L12

— Reply to this email directly or view it on GitHub https://github.com/sutoiku/formula.js/pull/32#issuecomment-65535061.