FLTK 1.3.3 indentation fixed fluid version

[akab...@gmail.com]

Hey guys,

I made some little modifications on the source code of fluid to get a better indentation and eventually switch from a double space indentation to a tab indentation easily. I share this here in case someone wants it.

Here is the detail of what I changed :
- each '{' begins at the begining of a new line (except for inlined {} )
- you can easily change the number of space/tab used for indentation by commenting or not the line 96 of file code.cxx:
#define USE_TAB_INDENT // comment this line for space indentation, uncomment for tab indentation
and changing the value of "indent_inc" just bellow (don't forget to update consequently the number of chars in the string "space").
- callbacks' code can handle multilined code's indentation properly and doesn't put a ';' after a final '}' if any.
- inlined class methods don't get a ';' after the final '}'.

Enjoy ;)

[Greg Ercolano]

Sounds neat -- I advise however to submit patches through the STR system,
as patches are easily lost here mixed in with the group.

From the main fltk.org page, click on "Bugs & Features", then click
"Submit Bug or Feature Request", and fill out the form, setting Priority
to "Request for Enhancement". You can attach your file in the form
("Attach File:").

If you're posting a complete copy of the FLTK files (instead of a patch),
then be sure to tell us what version of fltk the file is from, and if svn,
what svn revision. This is because sometimes the code changes radically from
release to release, making it hard to tell what changes are yours and what
changes are ours.

[Akabane87]

Done : STR #3210

[Improvised Be]

Hi, Akabane87

Very thankful for your sharing, I really like it.

I haven't got the new version from fltk.org's official published version which include the function you submitted.
Might I share this version of fluid update on github.com? or you may submit it to some open source version control web station, so I can fork?

I.B

[MacArthur, Ian (Selex ES, UK)]

I missed the beginning of this thread; is the objective to adjust fluid's generated code to be a bit prettier laid out?

 

If so, I'm not sure I'd be convinced that was useful, really; fluid's output is meant to be read by the compiler, not by humans, so it doesn't matter how "pretty" it is so long as it is valid syntax...

The little formatting (such as it is) that fluid does is, I think, meant to more or less follow the fltk coding style, which isn't much like the style I use...!

 

For what it is worth, if I do want to work on fluid-generated code later, I generally just pass it through astyle or indent (or sometimes even uncrustify depending on what mood I'm in!) to make it look more human-readable and pretty.

 

 

 

 

 

 

Hi, Akabane87

Very thankful for your sharing, I really like it.

I haven't got the new version from fltk.org's official published version which include the function you submitted.
Might I share this version of fluid update on github.com? or you may submit it to some open source version control web station, so I can fork?

I.B

On Saturday, March 21, 2015 at 11:23:37 AM UTC-5, Akabane87 wrote:

 

Selex ES Ltd
Registered Office: Sigma House, Christopher Martin Road, Basildon, Essex SS14 3EL
A company registered in England & Wales.  Company no. 02426132
********************************************************************
This email and any attachments are confidential to the intended
recipient and may also be privileged. If you are not the intended
recipient please delete it from your system and notify the sender.
You should not copy it or use it for any purpose nor disclose or
distribute its contents to any other person.
********************************************************************

[Greg Ercolano]

On 08/04/15 03:57, MacArthur, Ian (Selex ES, UK) wrote:

I missed the beginning of this thread; is the objective to adjust fluid's generated code to be a bit prettier laid out?

 

If so, I'm not sure I'd be convinced that was useful, really; fluid's output is meant to be read by the compiler, not by humans, so it doesn't matter how "pretty" it is so long as it is valid syntax...

The little formatting (such as it is) that fluid does is, I think, meant to more or less follow the fltk coding style, which isn't much like the style I use...!

 

For what it is worth, if I do want to work on fluid-generated code later, I generally just pass it through astyle or indent (or sometimes even uncrustify depending on what mood I'm in!) to make it look more human-readable and pretty.

    Pretty sure some of this patch is useful, in that it prevents compiler warnings.

    There's an STR open (STR #3210) to address this patch, it's assigned to me, investigating.

    If I commit anything, it'll probably be #ifdef'ed out initially so that it can be enabled for testing to be properly vetted.

    I agree though, Ian, on not changing e.g. bracing style and perhaps even indent levels.

    The code only needs to be human readable for the purposes of interpreting errors from the compiler, and not to be in every possible desirable human coding format.

    Indeed, this is what code reformatting tools like unix indent(1) are for, which has plenty of options for different coding style preferences. This can be coded into the user's Makefile (invoked by fluid) as a pre-process to the compiler, so that the code
    compiled is first reformatted, so that error messages follow the reformatted code.

    I agree, I don't think fluid should deal with code beautification options, and should only be concerned with generating compilable code. No code it generates would keep everyone happy, so best to standardize on one format (fltk CMP, for consistency).

    Preferences in fluid might be a bad idea, too, since preferences for this can affect line numbers in error messages (consider two people getting different compiler results due to these preferences)

    FWIW, the list of issues the OP's patch addresses are:

---

* each '{' begins at the begining of a new line (except for inlined {} )

  --I'm not sure I'll carry over this feature; it's intentional our code
    generate K&R style "floating" braces. If we do support this, it would
    have to be with a preference I think. Ditto for Tab vs. Spaces.

    I think for proper support of "flexible" output formats, one should
    post-process fluid's generated code using a proper code beautifier
    like unix indent(1) which has numerous code formatting options.

* you can easily change the number of space/tab used for indentation by

  commenting or not the line 96 of file code.cxx:

  #define USE_TAB_INDENT // comment this line for space indentation, uncomment for tab indentation

  ..and changing the value of "indent_inc" just bellow (don't forget to

  update consequently the number of chars

in the string "space").

  --This as well should probably not be implemented.

    As you say Ian, fluid's code shouldn't need to be in various human coding styles;
    if a user wants to reformat fluid's generated code, they can and should use
    indent(1).

    Not having such a feature also keeps fluid's internal code trim; we don't want
    to have to support all possible human code formatting techniques.. It's hard enough
    to generate proper code for the compiler.

- callbacks' code can handle multilined code's indentation properly and doesn't put a ';' after a final '}' if any.

  --Changes that prevent compiler warnings due to unwanted trailing ;'s
    is a good thing. Need to see what "indentation properly" means -- we should
    definitely fix /inconsistent/ indenting, but not provide coding style options
    that programs like indent(1) can supply anyway.

- inlined class methods don't get a ';' after the final '}'.

  -- Ditto, sounds good if it avoids undesirable trailing characters
     that may generate compiler warnings.

---

[Akabane87]

Not that I want to defend my fix or anything like that but I initially made this indentation mod because it was broken with most of multilined user code boxes in fluid. Generally the first line was not indented as the others. So I made this "fix" to fix the issue and in the same time let anyone choose the number of space or tab to use as indentation level. I also chose to use the coding style I usually use in my projects ({ at the begining of a new line) for obvious personnal reasons ;).

Feel free to grab any or none of these modifications I did. As I already said I did this first for me and then made it convenient to modify to anyone wanting to change the indentation style.

And to answer to this comment :

"
If so, I'm not sure I'd be convinced that was useful, really; fluid's output is meant to be read by the compiler, not by humans, so it doesn't matter how "pretty" it is so long as it is valid syntax...
"

Debugging a badly indented code you wrote correctly indented in fluid is something super-annoying and frustrating. Given the small amount of code I wrote to fix it, I thing it was worth doing it and I'm actually still using and enjoying it every day I use fluid. For sure using fluid fasten everything, but it also brings you to ommit lots of things accidentally and create bugs. So when time comes to debug this generated code, it's actually easier to debug if it is "pretty".

[Hannu Vuolasaho]

When Fluid generates code, it also leaves trailing white space on
empty lines. IIRC this happens on empty line custom function. I can't
verify right now if the fl-file white space fix also fixed this.
Anyway this wasn't big issue for me as generated sources don't end up
in my git repo.

+1 for pretty output because code is another view to widget designed
in fluid and sometimes helps understanding why something doesn't work.

Hannu Vuolasaho

[Greg Ercolano]

Yes, agreed inconsistent (badly) indented code should be fixed;
fluid should generate code that is consistent with itself, so we'll
take any fixes for that.

And your mods for removing trailing ;'s are good too; some compilers
likely warn about extraneous ;'s.

But probably won't take mods that change fluid's code generation style
(for instance to try to match e.g. the user's hand-crafted code via
New -> Code -> Code), as there's too many possible coding styles.
And for each new coding style, many parts of fluid's code would need
to be changed.. it's hard enough getting fluid to generate code that
compiles on all compilers.

And how far is "too far" for supporting coding styles, before we've
basically implemented all the features of indent/astyle.

I'd offer the best way to get code from fluid where it matches a particular
coding style is run it through indent/astyle first before compiling.
You can make this a single keypress in fluid (Alt-G) to invoke a Makefile
target that takes fluid's generated .cxx/.h files, runs them through indent/astyle,
and then compiles and runs the app, to make the process automatic.

[Greg Ercolano]

On 08/04/15 10:13, Hannu Vuolasaho wrote:
> When Fluid generates code, it also leaves trailing white space on
> empty lines. IIRC this happens on empty line custom function. I can't
> verify right now if the fl-file white space fix also fixed this.
> Anyway this wasn't big issue for me as generated sources don't end up
> in my git repo.

Yes, we should get rid of things like trailing white.

Trouble is when we make fixes like this that change fluid's
generated code, it affects diffs between old and new
fluid-generated code. (Normally one would diff the .fl files)

> +1 for pretty output because code is another view to widget designed
> in fluid and sometimes helps understanding why something doesn't work.

+1 for consistent "proper" code (no trailing spaces, consistent indenting
for auto-generated code)

-1 for trying to match specific coding styles, or to try to basically
do all the features that tools like indent/astyle/etc provides. Those
apps can be used in the user's Makefile to reformat the fluid generated
code just before compiling.

Or at least that's my evolving opinion (Ian's too I think).
Would like to hear from other devs as well, though.

[Albrecht Schlosser]

+1 on everything you (Greg) wrote (don't want to repeat everything).

I agree that the code should be indented in a sensible way, but not by
trying to enforce a particular coding style. IIRC I saw code that was
indented differently WRT the first and all following lines, i.e. the
first line was more or less indented than all other lines. That's
certainly something we could fix.

ISTR that it is documented somewhere that fluid generates "human
readable code" (but not to what extent, WRT formatting).

Also, fluid should not *ADD* trailing whitespace to code, but I wouldn't
mind writing trailing whitespace if the user-written (callback) code
/contained/ trailing whitespace. It's up to the user to fix the code, we
shouldn't try to "repair" this.

FWIW: there's a fix for trailing whitespace in fluid (.fl) files in svn
r 10782 (STR #3239). Maybe a similar fix would help here as well, but I
have no idea where to look.

[Greg Ercolano]

On 08/04/15 11:41, Albrecht Schlosser wrote:
> Also, fluid should not *ADD* trailing whitespace to code, but I wouldn't
> mind writing trailing whitespace if the user-written (callback) code
> /contained/ trailing whitespace. It's up to the user to fix the code, we
> shouldn't try to "repair" this.
>
> FWIW: there's a fix for trailing whitespace in fluid (.fl) files in svn
> r 10782 (STR #3239). Maybe a similar fix would help here as well, but I
> have no idea where to look.

Yes, I noticed STR #3239 re: trailing white in .fl files was Hannu's
as well, and at first I thought he was talking about that.. but sounds
like Hannu brings up a /different/ issue re:trailing white in .cxx/.h code.

I'll look into seeing if I can solve that as part of STR #3210.

Agreed on not adjusting user supplied code (New -> Code -> Code) of
trailing white, only fixing fluid gen'ed code.

There is at least once instance I've seen where trailing white
is relevant:

// Ascii art:
// /|\
// |
//

..if there isn't trailing white after that trailing backslash,
the compiler thinks it's a multiline comment (not allowed).
Though In such cases I usually include non-white trailing char
that isn't an escape char, e.g. this solves it:

// Ascii art: //
// /|\ //
// | //
// //

[Albrecht Schlosser]

On 04.08.2015 21:54 Greg Ercolano wrote:

> There is at least once instance I've seen where trailing white
> is relevant:
>
> // Ascii art:
> // /|\
> // |
> //
>
> ..if there isn't trailing white after that trailing backslash,
> the compiler thinks it's a multiline comment (not allowed).

I'd really be surprised if that was really an error, although I'm not a
C/C++ "guru". Likely a compiler error (?).

Anyway, I tested it on my Linux gcc (g++) 4.8.4, and there was no
warning or error message.

Do you still know which compiler found an error in that code?

[Hannu Vuolasaho]

I checked the own function whitespace empty line status. I'm running
SVN whch has STR3210 fix.
Last update I pulled is r10807 | AlbrechtS | 2015-07-21 16:55:13 +0300

cat -E generated.cxx showed in function with empty line:
intToStr(progs, buffer,10);$
$
progBox->copy_label(buffer);$
Those lines have two spaces before end of the line.

In .fl line I have cat -E file.fl
}$
intToStr(progs, buffer,10);$
$
progBox->copy_label(buffer);$
And those lines doesn't have any leading space.

So this is issue with code generation but I couldn't find quickly
similar place to put one if as was done with STR3210.

2015-08-05 0:00 GMT+03:00 Albrecht Schlosser wrote:
> On 04.08.2015 21:54 Greg Ercolano wrote:
>
>> There is at least once instance I've seen where trailing white
>> is relevant:
>>
>> // Ascii art:
>> // /|\
>> // |
>> //
>>
>> ..if there isn't trailing white after that trailing backslash,
>> the compiler thinks it's a multiline comment (not allowed).
>
>
> I'd really be surprised if that was really an error, although I'm not a
> C/C++ "guru". Likely a compiler error (?).

More likely feature. I have bee hit by that once and --std=something
triggered that behaviour.

Restyling user's code would be also stupid but I think right solution
would be notify when
generating code when found trailing white space.

If custom code editor has line numbers turned on this could be enhanced with
minimal UI change. There is already popup saying wrote cxx and h files.
There could also be button "Found trailing white space" or something similar
which closes popup and opens non-modal browser listing functions and
line numbers
where trailing white space was found. Or maybe even the line itself
and something to click
to remove the white space and button to rewrite code. This feature
needs some book keeping in
code generation or change in data types.

Hannu Vuolasaho

[Greg Ercolano]

On 08/04/15 14:00, Albrecht Schlosser wrote:
> On 04.08.2015 21:54 Greg Ercolano wrote:
>
>> There is at least once instance I've seen where trailing white
>> is relevant:
>>
>> // Ascii art:
>> // /|\
>> // |
>> //
>>
>> ..if there isn't trailing white after that trailing backslash,
>> the compiler thinks it's a multiline comment (not allowed).
>
> I'd really be surprised if that was really an error, although I'm not a
> C/C++ "guru". Likely a compiler error (?).
>
> Anyway, I tested it on my Linux gcc (g++) 4.8.4, and there was no
> warning or error message.

It's a warning with -Wall not an error (see below)
but we don't want warnings either.

> Do you still know which compiler found an error in that code?

The default gnu compiler on my sci linux 6.3 machine, which is 4.4.6:

Tue 08/04/15 16:39:04 /eagle/net/tmp
[erco@zoar] 8 : cat foo.cxx
//
// foo \
//
//
int x = 12;

Tue 08/04/15 16:39:11 /eagle/net/tmp
[erco@zoar] 9 : g++ -Wall foo.cxx -o foo -c
foo.cxx:2:1: warning: multi-line comment

Tue 08/04/15 16:39:21 /eagle/net/tmp
[erco@zoar] 10 : g++ --version
g++ (GCC) 4.4.6 20120305 (Red Hat 4.4.6-4) <--
Copyright (C) 2010 Free Software Foundation, Inc.



[erco@zoar] 3 : cat foo.cxx
//
// foo \
//
//
int x = 12;

[Albrecht Schlosser]

Oh, yes, I see ... with -Wall I can reproduce the warning. It's really
weird: I added a few spaces after the '\', but the warning remained.
Another printing char after the spaces fixed it.

[100%] Building CXX object test/CMakeFiles/hello.dir/hello.cxx.o
/home/albrecht/git/fltk/test/hello.cxx:24:3: warning: multi-line comment
[-Wcomment]
// /|\
^
Linking CXX executable ../bin/examples/hello
[100%] Built target hello
[ubuntu-64-nb: ~/git/fltk/build/codelite/Debug] $ g++ --version
g++ (Ubuntu 4.8.4-2ubuntu1~14.04) 4.8.4

You can learn something new every day...

(although I still believe that this compiler warning is wrong). A
comment is a comment ...

[Greg Ercolano]

On 08/04/15 15:11, Hannu Vuolasaho wrote:
> I checked the own function whitespace empty line status. I'm running
> SVN whch has STR3210 fix.

There's no commits in STR #3210 yet,
Just patches posted to the STR.

So you won't see anything new by just upgrading to svn current.

To see the OP's original changes, start with 1.3.3 and then build
his version of fluid in the rar file he attached.

Or, if you want to use svn current, I made a merge of his code
as a patch to svn current; in STR 3210 see:

fluid-indent-r10767_v2.patch

Just apply the patch, and rebuild fluid with the FLUID_STR_3210 macro
defined, and it should build with the OPs mods (if I wrangled the merge
correctly).

To do this, I cheated by just running make as:

cd fluid
make clean
make OPTIM=-DFLUID_STR_3210=1

..and then you can run fluid to test the patch.

I didn't test my merge yet, just got it to build.
But feel free to play with it if you like and weigh in.

[MacArthur, Ian (Selex ES, UK)]

> Or at least that's my evolving opinion (Ian's too I think).
> Would like to hear from other devs as well, though.

Yes; sounds about right to me.


As an aside, I use code beautifiers fairly often (even tried to write my own once) and I have yet to find one that actually does what I'd want it to do (hence the attempt to write my own...)

I do not think trying to add a "generic" code beautifier to fluid is a worthwhile objective, as the complexity is just hideous... A simple beautifier that creates some "fltk-canonical" format might be more achievable, but even there the issue of what to do with user-written code in the callback is hard to resolve.

It's a tricky problem.




I note that some more recent languages do specify a canonical format and provide tools to style random code in that format.
I'm thinking of google-go, and what some of the Python PEP stuff does, and ISTR the Plan9 C-compiler did that too for example (the styling in Plan9 C and Go! are presumably driven by Rob Pike, at a guess?)

Anyway, neither C nor C++ have such a canonical format, so styling is (a) hard to automate and (b) a seriously controversial bike-shed issue.

I do not think we can realistically fix that here!

Cheers,
--
Ian