Page 5 of 5

Re: Require spaces around binary ops

PostPosted: Mon Oct 14, 2013 5:16 am
by hopscc
Anyone had any more thoughts about forcing need for spaces around some binary ops ?

Anyone tried out the minimal space requiring patch on ticket:341
to see if doing so is either
  • glaringly advantageous
  • good
  • bearable
  • bad
  • annoying
  • needless nitpick whining
?

Re: Require spaces around binary ops

PostPosted: Mon Oct 14, 2013 12:31 pm
by nerdzero
I've been meaning to try this but just haven't found the time. But since you invited some nitpicking... :)

Looking at the code in the patch, would this break the ability to do line breaks on binary operators like this?

someString = _
"a really long line of text " +
"and another really long line of text"


Also, the patch wouldn't be able to compile itself, right? Line 747 of http://cobra-language.com/trac/cobra/at ... Wrap.patch is:

if _parenDepth >0, _parenDepth -= 1


I'm going to try this and the ?. operator patch soon though. I really keep running into instances where ?. would be nice.

Re: Require spaces around binary ops

PostPosted: Tue Oct 15, 2013 5:29 am
by hopscc
(Hmm - NoteToSelf - must remember to not use 'nitpick' in a posting referring to a patch :P )

re breaking implied line extension on trailing binary ops.
- ideally/realistically - no
- For the example you give - no
(applies to ASSIGN and EQUALS only)
- generically (as written for this prototype if extended for all suitable binary ops) probably
I coded the trailing test for space or tab (not nl) due to some uncertainty as to exactly what is applied wrt patches for implied line extension...

re patch compile itself:
The patch will compile itself (GREATER_THAN is not one of the binary ops checked in this patch)

The patch does not address any of the failing tests due the patch change which includes the patched compiler building itself...
It doesnt cover forcing this on any other binary op than ASSIGN and EQUAL.

The idea is/was for anyone interested and able to apply the patch, rebuild the compiler and try out the effect allowing some experiential data to the opinions given so far.
- This is NOT a patch to push as is into the source tree - = ?experimental/test/try-out/prototype-feature patch?.

Try out effect and tweak as see fit for whatever edge cases make sense or seem most 'comfortable'
(in my case relax the restriction for this check on ops/expressions inside parens).

Re: Require spaces around binary ops

PostPosted: Tue Oct 15, 2013 10:07 am
by nerdzero
Ah, I see. I think it's the right call to not require spaces inside parenthesis...or at least for assigning a default value to a method parameter. Okay, I'll actually try this before more needless whining from me :D

Re: Require spaces around binary ops

PostPosted: Thu Oct 31, 2013 10:50 am
by nerdzero
Hops,

Is Spacewrap.patch the only patch I need? I'm getting the following errors when trying to install Cobra after application of the patch:

Code: Select all
==== Build Cobra.Core library

run: cobra.exe -build-standard-library -debug -turbo -out:Cobra.Core.dll -key-file:Cobra.Core/Cobra.Core.snk  Cobra.Core/AssemblyAttrs.cobra
   : Cobra.Core.tmp/CobraFrame.cobra(94,17): error: Need at least one whitespace character around the operator ("==" (EQ)).
   : Cobra.Core.tmp/Extensions.cobra(107,26): error: Need at least one whitespace character around the operator ("==" (EQ)).
   : Cobra.Core.tmp/Extensions.cobra(108,27): error: Need at least one whitespace character around the operator ("==" (EQ)).
   : Cobra.Core.tmp/Extensions.cobra(109,27): error: Need at least one whitespace character around the operator ("==" (EQ)).
   : Cobra.Core.tmp/Extensions.cobra(110,31): error: Need at least one whitespace character around the operator ("==" (EQ)).
   : Cobra.Core.tmp/ExtendIEnumerable.cobra(46,14): error: Need at least one whitespace character around the operator ("==" (EQ)).
   : Cobra.Core.tmp/ExtendList.cobra(293,35): error: Need at least one whitespace character around the operator ("==" (EQ)).
   : Cobra.Core.tmp/Misc.cobra(38,68): error: Need at least one whitespace character around the operator ("=" (ASSIGN)).
   : Cobra.Core.tmp/Misc.cobra(45,60): error: Need at least one whitespace character around the operator ("=" (ASSIGN)).
   : Cobra.Core.tmp/MultiList.cobra(337,20): error: Need at least one whitespace character around the operator ("=" (ASSIGN)).
   : Compilation failed - 10 errors, 0 warnings

Re: Require spaces around binary ops

PostPosted: Fri Nov 01, 2013 1:44 am
by hopscc
Yes.
Its the only patch I made.

As noted previously it doesnt include any changes to make the compiler build itself, (and by implication the libraries, tests, How-tos, samples)
and I never tried it on the installer (obviously) -
looks like its working tho and space wrapping both sides of EQ currently isnt ubiquitous.

The idea is to rebuild the compiler and run it (locally) on whatever you want to test it out on.
(extend the ops covered if want to try out against more binary ops).

If theres any consensus on it being worthwhile I'll probably make a full patch that covers the compilers,
tests, installer and uncle-tom-cobleigh-and-all suitable for applying against the source tree.

Re: Require spaces around binary ops

PostPosted: Fri Nov 01, 2013 6:00 am
by nerdzero
As noted previously it doesnt include any changes to make the compiler build itself, ...

Right, I forgot you posted that. Sorry. I'll have to play with this some more then. I want to see how this looks inside MonoDevelop with squiggly lines under the errors.

This is line 94 of CobraFrame by the way:
label = if(j==0, 'args:', '')

I thought it was supposed to relax the check inside parenthesis. I haven't had my coffee yet so maybe I am missing something obvious here.

Re: Require spaces around binary ops

PostPosted: Fri Nov 01, 2013 6:27 am
by nerdzero
Okay, got some coffee. "if(" is a single token which is why it isn't picking up the parenthesis. :)

Re: Require spaces around binary ops

PostPosted: Sat Nov 02, 2013 4:11 am
by hopscc
Probably missed that construct.
Whatever chk i put in for parens should be extended to include the 'if(' token (OPEN_IF)