Question about contibuting code

[gambiteer]

I'm talking to a student starting to use LambdaNative to do some GUI stuff, and he started with Conway's Game of Life, which has a lot of boxes (either black or white) and he noticed that his program was going quite slowly.

While investigating, I think I found some quadratic behaviour (funny, my browser just marked "behavior" as a spelling mistake) in glgui-widget-add and glgui-widget-delete. That's probably not the reason the student's program is slow, but it's something.

So, a number of questions:

Are you looking for people to suggest changes to the code?

If so, do you have a required coding style?

And is there any way to discuss the code outside of starting a new issue?

Brad

For these routines I would suggest

(define (glgui-widget-add g . args)
  ;; Add a new widget to the end of g's widget-list
  (let* ((tmplist
          (let loop ((a args)  ;; collect pairs of args into an association list
                     (r '()))  ;; in reverse order (order doesn't matter in list->gtable)
            (cond ((null? a)
                   r)
                  ((null? (cdr a))
                   (error "glgui-widget-add: arguments must come in pairs: " args))
                  (else
                   (loop (cddr a)
                         (cons (cons (car a) (cadr a))
                               r))))))
         (entry
          (list->gtable tmplist)))
    (let ((widget-list (glgui-get g 'widget-list '())))
      (glgui-set! g 'widget-list (append widget-list (list entry))))
    (let ((widget-count (glgui-get g 'widget-count 0)))
      (glgui-set! g 'widget-count (fx+ widget-count 1)))
    entry
  ))

(define (glgui-widget-delete g w)
  ;; remove all instances of w from g's widget-list
  (let ((l (glgui-get g 'widget-list))
        (c (glgui-get g 'widget-count)))
    (glgui-set! g 'widget-list
                (let remove-w ((ws l))   ;; remove w while maintaining list order
                  (cond ((null? ws)
                         ws)
                        ((equal? (car ws) w)
                         (remove-w (cdr ws)))
                        (else
                         (cons (car ws)
                               (remove-w (cdr ws)))))))
    (glgui-set! g 'widget-count (fx- c 1))
  ))

Tested lightly with the following interaction inside a REPL in DemoRedSquare:

Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
----
LambdaNative REPL
DemoRedSquare 1.0
Built 2016-10-14 15:34:15 (00h 01m 32s ago)
> gui
#<table #2>
> (glgui-get gui 'widget-list)
(#<table #3>)
> (glgui-box gui 0 0 10 10 Blue)
#<table #4>
> (glgui-get gui 'widget-list)
(#<table #3> #<table #4>)
> (glgui-box gui 10 10 10 10 Green)
#<table #5>
> (glgui-get gui 'widget-list)
(#<table #3> #<table #4> #<table #5>)
> (glgui-widget-delete gui #4)
> (glgui-get gui 'widget-list)
(#<table #3> #<table #5>)
> 

[mgorges]

@gambiteer we'd love contributions in forms of pull requests. At the moment there is no place to discuss outside of issues, but you can post links to a Gist, which allow people to comment.

We should probably formalize our current coding style rules, but in a nutshell:
a) Indent with two spaces, no tabs
b) No trailing spaces at ends of lines
c) Match brackets indentation - in let's align the inner '(', in cond align the tests '('
d) If things don't fit in a line, break and continue indented only 2 positions
e) Sensible grouping is fine, particularly for closing brackets

[gambiteer]

On 10/14/2016 04:05 PM, Matthias Görges wrote:

We should probably formalize our current coding style rules, but in a
nutshell:
a) Indent with two spaces, no tabs

I have tabs turned off.

b) No trailing spaces at ends of lines

Got it.

c) Match brackets indentation - in let's align the inner '(', in cond
align the tests '('

OK.

d) If things don't fit in a line, break and continue indented only 2
positions
e) Sensible grouping is fine, particularly for closing brackets

To my knowledge, item (d) is not the default emacs indentation style for
Scheme.

To be honest, after loading the lambdanative .emacs mods I found it
difficult to write scheme code, I was so used to using to
auto-indent continuation lines. I uninstalled the lambdanative mods to
write the code I posted (as is evident).

Is there some way to massage my glgui-widget-add, for example, within
the default .emacs style, to make it suitable for the lambdanative code
base?

Brad

[mgorges]

@gambiteer Sorry nobody here uses Emacs, we are either vi or Eclipse people. I can review your suggestions tonight and merge them with my usual indentations - maybe that can server as a template to modify the emacs one based upon? I believe the current LambdaNative emacs thing is setup in make.sh#L1466 and is a compilation of other sources combined?

[mgorges]

As you asked how I would format it, here a patch I would propose to commit: widget.patch

When coming to these cond (but I do the same for other tests) if the result is short, I don't bother with separating it over three lines with ((test) / command / ) but put it all in one.

[gambiteer]

Thanks. I don’t know how to set up emacs with formatting like that.

Additionally, I still don’t quite “get” what visual language you want to obtain, e.g., having “cond” and the ending parentheses on their own lines seems to expand vertical space, while putting the test and the consequent on the same line seems to compress vertical space. I have to admit that I found the original code hard to understand. (For example, I thought at first that glgui-widget-delete reversed the order of the remaining widgets.)

If I propose another patch, I’ll try to format it more closely to how I understand your style to be.

Brad

[mgorges]

Hi @gambiteer, we'll try to sit down next week and come up with a few simple rules we'd like to follow. Personally, I think as long as it is sensible to read I am less fussed about formating and care more about content. Would you like me to commit the proposed patch you suggested? Thanks Matthias

[gambiteer]

On Oct 15, 2016, at 7:38 PM, Matthias Görges notifications@github.com wrote:

Would you like me to commit the proposed patch you suggested? Thanks Matthias

If you’re satisfied that it’s correct (I am), then please, go ahead.

Thanks.

Brad

[gambiteer]

On 10/14/2016 04:31 PM, Matthias Görges wrote:

@gambiteer https://github.com/gambiteer Sorry nobody here uses Emacs,
we are either vi or Eclipse people.

There's a set of Scheme formatting macros for vi/vim, by Dorai Sitaram:

https://github.com/ds26gte/scmindent

I've never used them, but I had a student who wrote several thousands
lines of Scheme code in vim, and he liked the formatting.

Brad

[mgorges]

Brad, I mainly do manual formatting, but use a very simple macro to enforce some things. Will have a look at your macros tomorrow:

mg@kanonia ~ $ cat .vimrc 
syntax on
set tabstop=2 shiftwidth=2 expandtab
map <F2> :retab <CR> :wq! <CR>
autocmd BufWritePre *.scm :%s/\s\+$//e

[ddunsmuir]

Just FYI, our summer student added a Style Guide to the wiki based on looking through our code.

[peterlew]

Whereas @mgorges implemented the proposed changes in commit 7a978c7, and much of the formatting discussion has been addressed in the Style Guide, this issue can be closed.