| | [ Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]
STR #3210
| Application: | FLTK Library |
|---|
| Status: | 1 - Closed w/Resolution |
|---|
| Priority: | 1 - Request for Enhancement, e.g. asking for a feature |
|---|
| Scope: | 3 - Applies to all machines and operating systems |
|---|
| Subsystem: | FLUID |
|---|
| Summary: | FLTK 1.3.3 indentation fixed fluid version |
|---|
| Version: | 1.3-feature |
|---|
| Created By: | AlainBandon |
|---|
| Assigned To: | matt |
|---|
| Fix Version: | 1.4.0 |
|---|
| Fix Commit: | 1c962bf5e26d3883bfc804b111d28080036d4feb |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
Trouble Report Comments:
| |
| #1 | AlainBandon
11:57 Mar 21, 2015 |
| From google group :
(modified code from fluid's sources from fltk 1.3.3)
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 ;) | |
| |
| #2 | greg.ercolano
20:41 Aug 03, 2015 |
| Assigning this to myself.
I think what I'll do is first make a patch against 1.3.x svn current
(OP's patch is against 1.3.3).
Will also make the code CMP compliant (OP: fltk source code indenting
is 2 space and K&R style bracing).
Will probably then commit with the new code #ifdef'ed so that it
can be enabled by people that want to test it, and once fully vetted,
can be fully committed. Comments from devs on this approach? | |
| |
| #3 | greg.ercolano
20:42 Aug 03, 2015 |
| Oh, and by #ifdef'ed, I mean something like:
#ifdef FLUID_STR_3210
// NEW
write_c("\nFl_Menu_Item %s::%s[] = \n{\n", k, menu_name(i));
#else
// OLD
write_c("\nFl_Menu_Item %s::%s[] = {\n", k, menu_name(i));
#endif | |
| |
| #4 | greg.ercolano
22:10 Aug 03, 2015 |
| Attaching untested v1 patch against r10767 current
with CMP mods. No logic changes.
Ran out of time tonight; will test compile with + without -DFLUID_STR_3210
and will follow up. | |
| |
| #5 | greg.ercolano
08:49 Aug 04, 2015 |
| Ian made some comments regarding this patch that I agree with; see:
https://groups.google.com/d/msg/fltkgeneral/kQbQw9ynkqU/_zJpLT-aJAAJ
Relevant excerpts from Ian's comments:
"..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."
I tend to agree with this: not make fluid into the unix indent(1) tool.
You can invoke indent (or similar) in your Makefile that fluid can invoke
(with e.g. Alt-G) to automatically reformat code before building, so that
you can see the code in whatever format you like during code development cycles.
We should however take patches that addresses unwanted trailing characters
(like trailing ;'s where they're not needed) or addresses any inconsistent indenting. | |
| |
| #6 | greg.ercolano
11:30 Aug 04, 2015 |
| As part of this STR, let's see if we can solve any trailing white space
issues in fluid's code generation. Hannu mentioned in today's thread
he's seen trailing white in fluid's generated code (sounds different
than STR #3239 which addressed trailin white in .fl saved files) | |
| |
| #7 | greg.ercolano
16:55 Aug 04, 2015 |
| Attached a v2 patch; small mod to my svn current merge. | |
| |
| #8 | greg.ercolano
18:06 Aug 07, 2015 |
| Small bug in my v2 port -- there was an ifndef instead of an ifdef
that broke fluid..
Uploading v3 here with fix. | |
| |
| #9 | AlbrechtS
16:50 Aug 13, 2015 |
| I took a look at the v3 patch (I didn't open the rar file at all). Here are my first comments:
(1) code.cxx: the two indent() functions are way too complicated (code duplication and redundant multiplications). This (untested code) could be a replacement, starting at line #117 of the patched code.cxx file:
#ifdef FLUID_STR_3210
// NEW
// Return indent level chars for specified indent level 'i'
const char* indent(int i) {
if (i>indent_max_level)
i = indent_max_level;
return spaces+((indent_max_level-i)*indent_inc);
}
// Return current indent level chars for global indentation
const char* indent() {
return indent(indentation);
}
#else /*FLUID_STR_3210*/
// OLD
...
I intentionally showed only the modified part. Full patch available on request.
The indent() function itself and its usage looks like a good idea to me, and the patch looks pretty good regarding its usage. I didn't test anything though, so I don't know if it is correct in all cases.
(2) code.cxx: I suggest to drop the option USE_TAB_INDENT, because this is not FLTK standard.
(3) As Ian wrote, I'm also not convinced that moving the opening '{' to the next line is a good idea (again: not FLTK standard). Furthermore, the code (as it is in the patch) would introduce new trailing whitespace. Greg's comment #3 shows a typical example (this is repeated all over the patch):
#ifdef FLUID_STR_3210
// NEW
write_c("\nFl_Menu_Item %s::%s[] = \n{\n", k, menu_name(i));
would put a space before '\n{' here ..^^^^ .
We should consider not to change this (stay with FLTK coding standard).
(4) There is some code I don't understand (yet?), and maybe the original patch was taken from an earlier version of the code, so that the patched version may be missing some recent changes. But I'm not sure about this, and it's too late now to give more info. I marked suspicious parts in my working copy so I can follow up with more info later.
Anyway, here is one example: in Fl_Widget_Type.cxx, lines #2054-2068 (after patch) seems to be completely rewritten (compared with the OLD code in lines #2071-2081). I don't see the equivalent code of this part:
// Only add trailing semicolon if the last line is not a preprocessor
// statement...
if (*p != '#' && *p) write_c(";"); | |
| |
| #10 | AlbrechtS
05:33 Aug 14, 2015 |
| Okay, let's see what I have now:
The original patch (rar file) was based on release 1.3.3, so there have been a few (small, but significant) modifications in fluid since. Matt added comments, and there was an STR related to avoiding "unused variable" compiler warnings and maybe more.
Some of these changes could be found in the "OLD" part of Greg's patch, but others had slipped through and would have been lost. I used git to apply the original patch to one branch (based on 1.3.3), merged the master branch (current svn) and compared this to another branch with Greg's patch (v3). After applying Greg's code formatting in "my" branch as well, I could clearly see the differences. I applied these to the patched svn version (with Greg's v3 patch), and the result is in the new patch file fluid-indent-r10831_v4.patch. I hope I didn't miss anything.
Note that I did NOT apply my simplification of the indent() functions.
The new patch (v4) is only a formal update of Greg's v3 patch, but all my concerns posted before still apply -- except that I believe that this patch is now correctly merged with current svn.
@Greg: I only tried to fix the missing parts (but not indent()). Please feel free to go ahead and work on the modified patch. I'm not going to do more for this STR in the near future. | |
| |
| #11 | AlbrechtS
01:55 Mar 10, 2019 |
| Changed "Subsystem" from "Core Library" to FLUID. | |
| |
| #12 | AlbrechtS
11:37 Dec 06, 2021 |
| This STR should probably be assigned to Matt because he's working on fluid.
@Matt: Greg and I worked on this STR (see comments and patches) but I'm not sure if everything is worth the effort. Sometimes it's good to be able to "read" the generated code with correct indenting (as a human), and indenting is already part of the fluid output, hence it could maybe be improved.
ISTR that I fixed output of TAB's in fluid after we disallowed tabs in source code which might affect the existing patch(es). | |
| |
| #13 | matt
13:40 Dec 09, 2021 |
| Partial fix of indenting callbacks correctly and being smarter about appending semicolons.
GitHub: a4b6175dbe4f613a690a09ad90b8df599e279bf5 | |
| |
| #14 | matt
19:09 Dec 10, 2021 |
| Fixed in Git repository. | |
[ Return to Bugs & Features ]
|
| |
