Jump to content
baka0815

[Patch] Formatter: more options for uses

Recommended Posts

We're styling the uses section the following way:

uses
  Windows
, SysUtils
, Classes
, Graphics
, Forms
;
  1. break after uses
  2. break after each unit (but before the comma)
  3. only indent the lines not starting with a comma ("Windows" in this example)
  4. break before the semi-colon

 

I created a patch and added it to the corresponding ticket: https://sourceforge.net/p/gexperts/feature-requests/154/

It adds two new options to the formatter (line-break page):
'After "uses"' and 'Before the comma'. The latter as a sub-option for 'Between every unit in "uses"' (like 'Except single lines').

'After "uses"' just adds a break after "uses", to start the "uses"-block.
"Before the comma" adds the line feed before the comma in the uses section instead of after it.
I also removed the fixme regarding NoIndentUsesComma and FeedEachUnit as it (currently) does exactly what I intended when I submitted the NoIndentUsesComma-patch some time ago.

 

I would like to see this patch merged or at least have some comments if there is something wrong/not as intended.

  • Like 1

Share this post


Link to post
42 minutes ago, baka0815 said:

I would like to see this patch merged or at least have some comments if there is something wrong/not as intended.

I have seen the ticket, but haven't yet found the time to actually look at the code. But I definitely will.

I'm grateful for any contribution to GExperts, in particular for patches that fix bugs or improve the tool.

  • Thanks 1

Share this post


Link to post

Thanks Thomas for your time!

 

If you need help looking at the patch or have questions, just ask here or at sourceforge.

Share this post


Link to post

I have just checked in a slightly modified version of your patch.

The modifications were:

  1. removed inline variable with type inference because this is not supported by Delphi 6
  2. fixed tab order issue in configuration dialog
  3. set the options to False in the Borland and DelForEx default configurations so they are not automatically turned on for existing configurations

Thank you for your contribution.

Share this post


Link to post

Thanks for including the patch!

You also asked for unit test on sourceforge - is there a guide or any other kind of help what to check before contributing a patch (like where to add unit tests or how to change the default configurations)?

So that I can incorporate that the next time?

Share this post


Link to post

Unfortunately not. At least I'm not aware of any and I never wrote one. It's possible that Eric wrote something when he was maintaining the project, but I'm not sure.

 

The part about Delphi 6 compatibility should be obvious, but on the other hand even I tend to forget it once in a while and find out the hard way when the code stops compiling.

Share this post


Link to post

Yeah, I'm sorry for using inline variables. They are so convenient and I like them that much, that I'm using them all the time since we upgraded to 10.x.

However I don't have Delphi 6 - I could install D2007 fwiw.

 

If I find the time I will create some unit tests for that functionality!

Share this post


Link to post
45 minutes ago, baka0815 said:

They are so convenient

They are indeed. I'm looking forward to using them one day when my main development environment will have caught up. Currently I'm stuck with Delphi 2007 compatibility at work, just to cater for the two remaining Windows XP computers our programs must run on (Yes, I could upgrade to Delphi XE<LastXpCompatible> but that won't help much.)

Share this post


Link to post
Posted (edited)

I just took a look at the unit tests and there are some tests currently failing.

Those are the CurlyHalfCommentEnd, MultilineFunctionDirective1 and MultilineFunctionDirective1 tests, but those are already broken since at least revision 3666 (17.10.2021), so I won't touch them.

 

Normally I would write a test with "FeedEachUnit=0" and another with "FeedEachUnit=1" to test this.

There are some settings which could affect other settings and I don't want to create 10 or so configurations or the same amount of "AdjustSettings"-methods.

 

Any help in this regard is appreciated.

Edited by baka0815

Share this post


Link to post
31 minutes ago, baka0815 said:

I just took a look at the unit tests and there are some tests currently failing.

Those are the CurlyHalfCommentEnd, MultilineFunctionDirective1 and MultilineFunctionDirective1 tests, but those are already broken since at least revision 3666 (17.10.2021), so I won't touch them.

Yes, they have been broken basically since forever and for the CurlyHalfCommentEnd one I don't really care much. But the MultilineFunctionDirective bug is really annoying. I must find some time to finally fix that.

 

32 minutes ago, baka0815 said:

Normally I would write a test with "FeedEachUnit=0" and another with "FeedEachUnit=1" to test this.

There are some settings which could affect other settings and I don't want to create 10 or so configurations or the same amount of "AdjustSettings"-methods.

The way the tests currently work is by checking the all the supplied settings with various input files and comparing them to the output files.

Testing one particular setting is currently not really part of the test. Maybe it would be easier for you to simple create a new TTestCase and, based on the default settings of the formatter, only run the two tests you mention.

 

btw: Thanks for caring about the unit tests. That's a first for any contributor.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×