ajaxorg / ace

Ace (Ajax.org Cloud9 Editor)
https://ace.c9.io
Other
26.69k stars 5.28k forks source link

bug in this.$detectNewLine #2677

Closed prettydiff closed 9 years ago

prettydiff commented 9 years ago
this.$detectNewLine=function(e){var t=e.match(/^.*?(\r\n|\r|\n)/m); and so on...

I am getting an error e is undefined

I can reproduce this error when I go to http://prettydiff.com/?m=beautify&l=csv and paste this code:

first, second, third
"cat""",dog,otherfdsas

I have verified the mode is set to "plain_text" which evaluates to "ace/mode/text". You can easily access both editors from the browser console as pd.ace.beauIn and pd.ace.beauOut

nightwing commented 9 years ago

This can happen if you call insert with non string value. When following steps you described above, I do not see this error with chrome 45

snobu commented 8 years ago

Getting the same error, i'll try to build a short repro, but i only replaced v1.1.9 with 1.2.2 and now i'm getting this.

prettydiff commented 8 years ago

For me the error was generated due to a defect in my own code where I was passing in an array instead of a string.

snobu commented 8 years ago

Nevermind, i was doing something silly.

    // Somewhere after knockoutjs does its thing
    // ... 
    // Blank out the editor before fetching new content
    viewModel.editText(null);

It decided it's time to start pointing fingers :) Replaced the null with an empty string and it's all fine and dandy again.

prettydiff commented 8 years ago

It might be wise to have better error messaging associated with this issue to educate the user, so that the user realizes their code mistake implementing this awesome tool.

jeremy50dj commented 8 years ago

HTTPS://regeneratemanbot.slack.com For my team or just go to HTTPS://slack.com This is what will be writing code so don't waist your time trying to figure out bugs. I will make snippets from any code to fix any program and take out the trash to. On Nov 24, 2015 1:28 AM, "Adrian Calinescu" notifications@github.com wrote:

Getting the same error, i'll try to build a short repro, but i only replaced v1.1.9 with 1.2.2 and now i'm getting this.

— Reply to this email directly or view it on GitHub https://github.com/ajaxorg/ace/issues/2677#issuecomment-159193605.

snobu commented 8 years ago

It looks like we've clearly hit the same bug here but passing in two different things. An array and a null.

PR proposal:

console.error('setValue() expects a String')

This has the advantage of not breaking current implementations

or

snobu commented 8 years ago

You see, sometimes i speak as if Javascript is strongly typed or as if Global Warming is fixed. But there's just enough capacity to fix one.

36520_sad_seal

I'll just leave this here.