jcberquist / commandbox-cfformat

A CommandBox module for formatting CFML component files.
MIT License
22 stars 10 forks source link

Alignment of code #35

Closed lmajano closed 4 years ago

lmajano commented 5 years ago

This is more of a question than an issue just yet. But there are many places where we do code alignment for readability. With cfformat I have no way to reproduce this. Instead it removes this code alignment. Is this something possible? Please see screenshots

image

image

jcberquist commented 5 years ago

I think aligning struct values would be possible within the current framework. Of course then you would get that alignment in every struct (that was formatted onto multiple lines).

Aligning consecutive variable assignments wouldn't really be possible within the current code printing. The best I think I could do is add a post initial format pass that uses regex to find those lines and aligns them. I am not sure how well that would work in general, but I think with your examples above it would work.

lmajano commented 5 years ago

I think aligning struct values would be possible within the current framework. Of course then you would get that alignment in every struct (that was formatted onto multiple lines).

Yes, I think this is something that would be very useful for our standards and readability of code for code reviews. We also apply this approach to method calls:

throw(
    message         = "This message",
    type            = "ThisException",
    extendedInfo           = "Hello",
    detail          = "This details"
);

Aligning consecutive variable assignments wouldn't really be possible within the current code printing. The best I think I could do is add a post initial format pass that uses regex to find those lines and aligns them. I am not sure how well that would work in general, but I think with your examples above it would work.

This to us would be a nice thing to have. We go into great extends to do it in our code bases for readability for consecutive assignments.

jcberquist commented 5 years ago

I have an attempt at this in v0.12.1. It relies on a simpler regex approach, so it remains to be seen how robust it is. I tested it on ModuleConfig.cfc from your cbstorages and it seemed to do alright there.

I added a setting to control this: "assignments.consecutive.alignment": (true|false). It is an all or nothing setting for now (and I am not sure how much more precise I can get). Any consecutive variables assignments, named function call arguments, or struct key values should ideally get aligned when it is set to true. I would appreciate testing and feedback on it.

lmajano commented 5 years ago

@elpete WOW!!!

lmajano commented 5 years ago

I have tested it but can't make it work :( Screen Shot 2019-09-10 at 10 10 13 AM

I am attaching the code ContentStoreCarousel.cfc.zip cfformat.json.zip

jcberquist commented 5 years ago

Thanks for the test. I need to update the regex that searches for assignments to allow for bracket notation: slide[ "imageURL" ].

jcberquist commented 5 years ago

I have published v0.12.3 which should work with the bracket notation.

lmajano commented 5 years ago

This is working, however we have another case where it's not, property definitions. Can you check this out. Screen Shot 2019-09-24 at 7 43 14 AM

Is this possible?

jcberquist commented 5 years ago

This should be possible, however, I think I will have to use a slightly different approach for this since it is the attributes that are being aligned here.

jcberquist commented 5 years ago

@lmajano, I have published v0.12.7 - now there are three settings:

"alignment.consecutive.assignments"
"alignment.consecutive.properties"
"alignment.consecutive.params"

Note that this is a breaking change, sorry! The previous setting assignments.consecutive.alignment is gone in favor of alignment.consecutive.assignments - I was trying to keep these related settings together.

This is one of the reasons I haven't gone to a 1.0.0 release; I want to be able to adjust setting names. I know it is inconvenient though, so again, sorry. But in that vein, if you think any of the settings could be better named, now would be a good time!

lmajano commented 5 years ago

No worries at all! Thanks for all that you do!

lmajano commented 5 years ago

Ok, seems it doesn't want to align this:

/**
     * Constructor
     */
    function init(){
        variables.siteKeywords    = "";
        variables.siteDescription = "";
        variables.cb_site_mail_server = "";
        variables.cb_site_mail_username = "";
        variables.cb_site_mail_password = "";
        variables.cb_site_mail_smtp = "25";
        variables.cb_site_mail_tls = "false";
        variables.cb_site_mail_ssl = "false";
        variables.populateData = true;
        variables.fullRewrite  = true;
        variables.rewrite_engine = "mod_rewrite";

        return this;
    }
jcberquist commented 5 years ago

@lmajano: Thanks! It wasn't supporting underscores in variables names, but it is now :)

elpete commented 4 years ago

Just a note that I think you can close this issue. The alignment seems (to me) to be working great in our repos.

jcberquist commented 4 years ago

Sounds good, thanks!