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

Using LF (\n) instead of CRLF (\r\n) causes some parsing errors. #396

Closed
lukaspj opened this Issue May 28, 2013 · 13 comments

Comments

Projects
None yet
5 participants
@lukaspj
Contributor

lukaspj commented May 28, 2013

Ran a tool on my code which, among other things, replaced all file endings with LF.
This resulted in a lot of parsing errors in some of the script (not all, just some of them 8 in total)

Example of an error:

core/scripts/client/audio.cs Line: 414 - syntax error
>>> Advanced script error report.  Line 414.
>>> Some error context, with ## on sides of error halt:
///                     string to pause sources on all channels.
/// @param %pauseSet    An optional SimSet which is filled with the paused 
///                     sources.  If not specified the global SfxSourceGroup 
///                     is used.
///
/// @deprecated
///
function sfxPause( %channels, %pauseSet )
{
## ##  // Did we get a set to populate?
   if ( !isObject( %pauseSet ) )
      %pauseSet = SFXPausedSet;

   %count = SFXSourceSet.getCount();
   for ( %i = 0; %i < %count; %i++ )
   {
      %source = SFXSourceSet.getObject( %i );

      %channel = sfxGroupToOldChannel( %source.getGroup() );
>>> Error report complete.

Complete list of files that gave an error:

  • core/scripts/client/audio.cs Line: 414 - syntax error
  • tools/editorClasses/scripts/projects/projectEvents.ed.cs Line: 27 - syntax error
  • tools/editorClasses/scripts/projects/projectInternalInterface.ed.cs Line: 131 - syntax error
  • tools/editorClasses/scripts/input/inputEvents.ed.cs Line: 27 - syntax error
  • tools/base/menuBar/menuBuilder.ed.cs Line: 193 - syntax error
  • tools/worldEditor/scripts/editors/terrainEditor.ed.cs Line: 36 - syntax error
  • tools/worldEditor/scripts/editors/terrainEditor.ed.cs Line: 36 - syntax error
  • tools/datablockEditor/datablockEditor.cs Line: 338 - syntax error

Windows8x64 MIT 3.0 with no changes except line endings.

@thevisad

This comment has been minimized.

Show comment
Hide comment
@thevisad

thevisad May 28, 2013

This is an issue with GITHUB and is a known issue with repositories on it.

thevisad commented May 28, 2013

This is an issue with GITHUB and is a known issue with repositories on it.

@lukaspj

This comment has been minimized.

Show comment
Hide comment
@lukaspj

lukaspj May 28, 2013

Contributor

No no it was after downloading with GitHub, initially it ran fine. Then I ran a tool that changed all line endings and then the script parsing errors came.

Should be fixed tho, while it's not an issue for Windows users, it might be more comfortable for linux users to be able to write their scripts with LF line ending.

Contributor

lukaspj commented May 28, 2013

No no it was after downloading with GitHub, initially it ran fine. Then I ran a tool that changed all line endings and then the script parsing errors came.

Should be fixed tho, while it's not an issue for Windows users, it might be more comfortable for linux users to be able to write their scripts with LF line ending.

@DavidWyand-GG

This comment has been minimized.

Show comment
Hide comment
@DavidWyand-GG

DavidWyand-GG May 28, 2013

Member

I've seen this in the past with line endings. Likely something with the TorqueScript compiler.

Member

DavidWyand-GG commented May 28, 2013

I've seen this in the past with line endings. Likely something with the TorqueScript compiler.

@thevisad

This comment has been minimized.

Show comment
Hide comment
@thevisad

thevisad May 28, 2013

Yeah, github changes things, you actually have to force it to not do it. We had the same issue in another GIT I run. It was causing script errors in that system as well making merges fail (on a windows box). I would be careful of any tool that does this and manually make the changes on the files using something like Notepad++ and replace statements.

thevisad commented May 28, 2013

Yeah, github changes things, you actually have to force it to not do it. We had the same issue in another GIT I run. It was causing script errors in that system as well making merges fail (on a windows box). I would be careful of any tool that does this and manually make the changes on the files using something like Notepad++ and replace statements.

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Mar 15, 2014

Contributor

This was observed in the linux work. I'll look at the compiler and see if it's an easy change.

EDIT: In CMDScan.l I'm seeing this:

[\r]        ;
[\n]        {lineIndex++;}

So I'm guessing the error occurs somewhere else, since this looks to me like it should handle both \n and \r\n. @lukaspj do all those errors occur in function definitions immediately after /// comment blocks?

Contributor

crabmusket commented Mar 15, 2014

This was observed in the linux work. I'll look at the compiler and see if it's an easy change.

EDIT: In CMDScan.l I'm seeing this:

[\r]        ;
[\n]        {lineIndex++;}

So I'm guessing the error occurs somewhere else, since this looks to me like it should handle both \n and \r\n. @lukaspj do all those errors occur in function definitions immediately after /// comment blocks?

@crabmusket crabmusket added the Bug label Mar 15, 2014

@lukaspj

This comment has been minimized.

Show comment
Hide comment
@lukaspj

lukaspj Apr 15, 2014

Contributor

@eightyeight Sorry, I completely forgot about this thread.

Yes this error occurs every place where you have something on this form:

///
function foo(%args)
{ 

It has to be

/// 

with nothing else on that line, i.e.

/// comment
function foo(%args)
{ 

does not trigger the error.

I went through every instance of /// comments and it's a consistent error, nicely spotted.

Contributor

lukaspj commented Apr 15, 2014

@eightyeight Sorry, I completely forgot about this thread.

Yes this error occurs every place where you have something on this form:

///
function foo(%args)
{ 

It has to be

/// 

with nothing else on that line, i.e.

/// comment
function foo(%args)
{ 

does not trigger the error.

I went through every instance of /// comments and it's a consistent error, nicely spotted.

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Apr 15, 2014

Contributor

Right. /// comment blocks seem to have a special rule in the grammar and evidently something's going wrong with it.

Contributor

crabmusket commented Apr 15, 2014

Right. /// comment blocks seem to have a special rule in the grammar and evidently something's going wrong with it.

@lukaspj

This comment has been minimized.

Show comment
Hide comment
@lukaspj

lukaspj Apr 16, 2014

Contributor

Hehe the TS grammar code is one thing that I don't feel like diving into atm lol..
I hope there is someone here who know the script system better than me.

Contributor

lukaspj commented Apr 16, 2014

Hehe the TS grammar code is one thing that I don't feel like diving into atm lol..
I hope there is someone here who know the script system better than me.

@crabmusket crabmusket added this to the 3.7 milestone Apr 16, 2014

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Apr 16, 2014

Contributor

Slated for 3.7 when we do the other script engine changes like James' refactor.

Contributor

crabmusket commented Apr 16, 2014

Slated for 3.7 when we do the other script engine changes like James' refactor.

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Oct 14, 2014

Contributor

@JeffProgrammer feel like taking a look at this, while you're talking about the TS grammar?

Contributor

crabmusket commented Oct 14, 2014

@JeffProgrammer feel like taking a look at this, while you're talking about the TS grammar?

@JeffProgrammer

This comment has been minimized.

Show comment
Hide comment
@JeffProgrammer

JeffProgrammer Oct 15, 2014

Contributor

Will take a look, but yes its not the line endings, rather a blank /// that does trigger it. Been wanting to tackle that eventually :P

Contributor

JeffProgrammer commented Oct 15, 2014

Will take a look, but yes its not the line endings, rather a blank /// that does trigger it. Been wanting to tackle that eventually :P

@JeffProgrammer

This comment has been minimized.

Show comment
Hide comment
@JeffProgrammer

JeffProgrammer Feb 6, 2015

Contributor

Looking into this...

Contributor

JeffProgrammer commented Feb 6, 2015

Looking into this...

@JeffProgrammer

This comment has been minimized.

Show comment
Hide comment
@JeffProgrammer

JeffProgrammer Feb 6, 2015

Contributor

don't know regular expressions much, but what I'm assuming is that the regular expression requires at least 1 character after the 3 slash marks in order for it to compile from what I see, the regular expression:

("///"[^/][^\n\r]*[\n\r]*)+ { return(Sc_ScanDocBlock()); }

Contributor

JeffProgrammer commented Feb 6, 2015

don't know regular expressions much, but what I'm assuming is that the regular expression requires at least 1 character after the 3 slash marks in order for it to compile from what I see, the regular expression:

("///"[^/][^\n\r]*[\n\r]*)+ { return(Sc_ScanDocBlock()); }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment