Re: [Denemo-devel] Denemo warnings and code cleanup

[Richard Shann]

On Tue, 2013-05-14 at 16:15 +0200, Éloi Rivard wrote:
> I pushed a first pass on the master branch.
I had to remove the private header file included in exportmidi.c as it
would not compile (clearly, we should not be using anything that is
private to libsmf anyway - what prompted you to include this?)
Also, it would not compile for GTK2 without a definition in bookmarks.c
which I have added.
I haven't had time to look at the code, but a first quick play with the
program didn't throw up any big problem.

>  I removed most of unused variable, unused static functions (not in
> generated files), implicit function declaration, untyped functions and
> vars warnings, plus some miscellaneous ones.
> 
> I didn't touched most of warnings related to gtk deprecation, pointer
> casting (gpoint to int, int to gpointer), and macros, for the moment.
> 
> I commented unused functions, and tagged theme with UNUSED. Could you
> grep this tag and check if function may be used later, or delete
> them ?
Yes, perhaps we should put this in the bug tracker.
> 
> It would be great to be able to compile with -Werror one day for more
> safety :)
> 
> 
> Another thing, view.c is 11 000 lines long! It is almost 10% of the
> src directory :) It would be great to split it in several other files.
> I can take a look, but if would probably be better if you start this,
> as I don't know the mechanisms behind those 11k lines. What do you
> think?

It has been on my conscience for several years. I can thing of three
major sections that should be separate:

1) All the scheme related code, especially everything for generating
scheme primitives.

2) All the code related to music snippets, internally usually called
rhythms.

3) The code relating to creating the main window (which is where the
name view.c came from).

But there are certainly others. Code for handling clicking on menus and
more.

I have always been daunted by this task. As you have found trying to
refactor the keymap code it is a high risk occupation. I am not sure if
I can honestly say that I have the courage to tackle it. Perhaps if we
created a map of the file, that would be a start...

Richard


> 
> 2013/5/14 Richard Shann <address@hidden>
>         On Tue, 2013-05-14 at 11:47 +0200, Éloi Rivard wrote:
>         > Hi,
>         >
>         > Could you have a look at scheme_create_timebase function in
>         view.c.
>         > The "if" statement emits a warnings, but I don't know how to
>         fix it.
>         >
>         > Should it be a double equals operator, or the affectation
>         before the
>         > "if" statement ?
>         
>         
>         Good work - it is actually an ! that is missing. I use the
>         idiom
>         if((a=b)) ... whenever I am tempted to assign and test in one
>         go. I have
>         fixed this line in git master, thanks for the detective work.
>         
>         Richard
>         
>         
>         >
>         > --
>         > Éloi Rivard - address@hidden
>         >
>         > « On perd plus à être indécis qu'à se tromper. »
>         >
>         
>         > _______________________________________________
>         > Denemo-devel mailing list
>         > address@hidden
>         > https://lists.gnu.org/mailman/listinfo/denemo-devel
>         
>         
> 
> 
> 
> -- 
> Éloi Rivard - address@hidden
>         
> « On perd plus à être indécis qu'à se tromper. »
>