search

Warzone2100 / old-trac-import

Archived Import of (old) Warzone 2100 Trac
0 stars 0 forks source link

widgets lib converted with astyle sample #3220

Closed wzdev-ci closed 9 years ago

wzdev-ci commented 12 years ago

resolution_closed type_patch (an actual patch, not a request for one) | by vexed

Using these options

suffix=none
style=allman
indent=tab
indent-cases
indent-namespaces
pad-oper
pad-header
unpad-paren
add-brackets
align-pointer=name
min-conditional-indent=0
indent-preprocessor
keep-one-line-statements
max-instatement-indent=8

Issue migrated from trac:3220 at 2022-04-16 08:58:20 -0700

wzdev-ci commented 12 years ago

vexed uploaded file 0001-Using-astyle-with-these-otpions.patch (159.4 KiB)

wzdev-ci commented 12 years ago

cybersphinx commented

Hm, looks like astyle can't do indent with tabs, align with spaces? And indent-switch isn't what we do now.

wzdev-ci commented 12 years ago

vexed commented

Replying to Warzone2100/old-trac-import#3220 (comment:1):

Hm, looks like astyle can't do indent with tabs, align with spaces? And indent-switch isn't what we do now.

Perhaps we need to use more than one tool, or do you got a config file for uncrustify that we can try ?

wzdev-ci commented 12 years ago

dak180 commented

I would be interested to see what (if anything) --patience did to the patch.

wzdev-ci commented 12 years ago

vexed commented

Replying to Warzone2100/old-trac-import#3220 (comment:3):

I would be interested to see what (if anything) --patience did to the patch.

Hmm ? http://astyle.sourceforge.net/astyle.html

wzdev-ci commented 12 years ago

dak180 commented

Replying to Warzone2100/old-trac-import#3220 (comment:4):

Replying to Warzone2100/old-trac-import#3220 (comment:3):

I would be interested to see what (if anything) --patience did to the patch.

Hmm ? http://astyle.sourceforge.net/astyle.html Not astyle; git format-patch --patience.

wzdev-ci commented 12 years ago

cybersphinx changed _comment0 which not transferred by tractive

wzdev-ci commented 12 years ago

cybersphinx changed _comment1 which not transferred by tractive

wzdev-ci commented 12 years ago

cybersphinx changed _comment2 which not transferred by tractive

wzdev-ci commented 12 years ago

cybersphinx changed _comment3 which not transferred by tractive

wzdev-ci commented 12 years ago

cybersphinx commented

ls *h *cpp|parallel "sed -i -e \"s/( /(/g\" -e \"s/ )/)/g\" {}; uncrustify --no-backup --replace {}"

For some reason uncrustify didn't remove the spaces in macro calls (ASSERT), so add some sed.

Edit: Updated to not add superfluous spaces in for loops.

Edit: For newlines at eof set nl_end_of_file to add instead of force.

wzdev-ci commented 12 years ago

cybersphinx uploaded file .uncrustify.cfg (62.7 KiB)

wzdev-ci commented 12 years ago

cybersphinx uploaded file uncrustify.patch (234.0 KiB)

wzdev-ci commented 12 years ago

vexed commented

for uncrustify.cfg,

# Add or remove newline at the end of the file
nl_end_of_file                           = force   # ignore/add/remove/force

# The number of newlines at the end of the file (only used if nl_end_of_file is 'add' or 'force')
nl_end_of_file_min                       = 1        # number
wzdev-ci commented 12 years ago

Cyp commented

Few types of changes I noticed that I didn't really like:

 // In many cases, think it looks better without space around * and /, since they have higher precedence. Relatively minor issue for me, though.
- a = (b + c*d)/2;
+ a = (b + c * d) / 2;
 // Ouch, my eyes! Will probably lead to a lot more confusion and counting brackets to figure out which matches which...
-   if (mousePressed(MOUSE_LMB) && (mx < x || mx > x + width || my < y || my > y + height))
+   if (mousePressed(MOUSE_LMB) && ((mx < x) || (mx > x + width) || (my < y) || (my > y + height)))
 // I think of the * as part of the type, so it looks strange to me if the * is far from the type (I expect one space before, no more, no less).
- SOME_TYPE *     a;
+ SOME_TYPE      *a;

I'd generally prefer W_FORMINIT const over const W_FORMINIT .

wzdev-ci commented 12 years ago

Per commented

Please remove: indent-switches indent-classes

Thanks :)

wzdev-ci commented 12 years ago

vexed edited the issue description

wzdev-ci commented 12 years ago

vexed commented

I just noticed, that we could also removed the UDWORD / SDWORD / UWORD / SWORD (and whatever else is MS specific) to be uint32_t / int32_t / uint16_t / int16_t as a clean up.

Everyone OK with that as well ?

wzdev-ci commented 12 years ago

vexed commented

BTW, astyle can't fix all the formatting issues that we found, that is why we were looking at uncrustify.

wzdev-ci commented 12 years ago

Per commented

We don't need to fix everything (at once). Also, I am not sure that changing types automatically is a good idea, since we should use 'int' where bit size is unimportant, and using bit-precise types gives an indication that bit length is important. Doing some search & replace by hand would probably be fast enough and give better results.

wzdev-ci commented 12 years ago

vexed commented

Fair enough, will leave that out then.

wzdev-ci commented 12 years ago

Per Inge Mathisen commented

Clean up crazy indentation in function.cpp to make it readable. Refs #3220

wzdev-ci commented 12 years ago

Per Inge Mathisen commented

Clean up crazy indentation in function.cpp to make it readable. Refs #3220

wzdev-ci commented 12 years ago

Per Inge Mathisen commented

Clean up crazy indentation in function.cpp to make it readable. Refs #3220

wzdev-ci commented 12 years ago

Per Inge Mathisen commented

Clean up crazy indentation in function.cpp to make it readable. Refs #3220

wzdev-ci commented 11 years ago

vexed commented

Since Per has already done a few... 2f5ff10fa12425348c732e17b0a13ef9d630ef98 3b766d7043d6de8f293a898d3286ada93eecda24 358efd0484a0c8cb9b5557085ad9a04968bc8799

... and so on.

So, this weekend, might as well branch master to messy, then run astyle on everything that is left in master.

We really need to get this done ASAP, so all the patches we get won't clash so bad, and they can fix them with astyle as well.

Per, are you using

--suffix=none --indent=tab --brackets=break --indent-switches --pad-oper --pad-header --unpad-paren --min-conditional-indent=0 --mode=c --indent-preprocessor --keep-one-line-statements --max-instatement-indent=8
wzdev-ci commented 11 years ago

Per commented

I use 

suffix=none
style=allman
indent=tab
indent-cases
indent-namespaces
pad-oper
pad-header
unpad-paren
add-brackets
align-pointer=name
min-conditional-indent=0
indent-preprocessor
keep-one-line-statements
max-instatement-indent=8

I am a bit wary of running everything through astyle. I've done quite a few more attempts to run files through astyle than I've committed, and often the noise seems to outweigh the gain. Also, if everything was astyle'd, a lot of my uncommitted patches would break, I fear.

However, I won't really object.

wzdev-ci commented 11 years ago

vexed commented

Replying to Warzone2100/old-trac-import#3220 (comment:19):

I use 

suffix=none
style=allman
indent=tab
indent-cases
indent-namespaces
pad-oper
pad-header
unpad-paren
add-brackets
align-pointer=name
min-conditional-indent=0
indent-preprocessor
keep-one-line-statements
max-instatement-indent=8

I am a bit wary of running everything through astyle. I've done quite a few more attempts to run files through astyle than I've committed, and often the noise seems to outweigh the gain. Also, if everything was astyle'd, a lot of my uncommitted patches would break, I fear.

However, I won't really object.

That is why we make a cruft branch, and in theory, you apply patches to that first, then you can astyle it, and then you can transfer said new patch with virtually no conflicts.

wzdev-ci commented 9 years ago

vexed changed status from new to closed

wzdev-ci commented 9 years ago

vexed changed resolution from ` toclosed`

wzdev-ci commented 9 years ago

vexed commented

already done.