Skip to content

New Feature: JS formatter and prettifier#917

Merged
tlimoncelli merged 6 commits into
DNSControl:masterfrom
jpbede:feature/fmt
Mar 2, 2021
Merged

New Feature: JS formatter and prettifier#917
tlimoncelli merged 6 commits into
DNSControl:masterfrom
jpbede:feature/fmt

Conversation

@jpbede
Copy link
Copy Markdown
Member

@jpbede jpbede commented Oct 28, 2020

I liked the idea to have a JS prettifier for dnsconfig.js but i would like to have it integrated into dnscontrol, so you don't need to install an other tool.

This is currently a PoC but i wanted to create a PR for it, so you can have a look at it :)

Fixes #874

@jpbede jpbede marked this pull request as draft October 28, 2020 07:34
@jpbede
Copy link
Copy Markdown
Member Author

jpbede commented Oct 28, 2020

Done a little test

From:

var DSP_POWERDNS = NewDnsProvider("powerdns", "POWERDNS", {
    'default_ns'        : [
                  'a.anydns.io.',
           'b.anydns.io.'
    ],
            'zonetype': 'native',
});
        var REG_CHANGEME = NewRegistrar("ThirdParty", "NONE");

    var REG_GANDI = NewRegistrar("gandi", "GANDI_V5");
                    var ips = require('ips.json');

                    if(true)
                    {
                    console.log("jj")
                    }

// load base file
// contains records which are shared across zones
    require('nameserver.js');
require('base.js');

// load zones
require_glob("zones/", false);
    require_glob("reverse/", false);

To:

var DSP_POWERDNS = NewDnsProvider("powerdns", "POWERDNS", {
    'default_ns': [
        'a.anydns.io.',
        'b.anydns.io.'
    ],
    'zonetype': 'native',
});
var REG_CHANGEME = NewRegistrar("ThirdParty", "NONE");

var REG_GANDI = NewRegistrar("gandi", "GANDI_V5");
var ips = require('ips.json');

if (true) {
    console.log("jj")
}

// load base file
// contains records which are shared across zones
require('nameserver.js');
require('base.js');

// load zones
require_glob("zones/", false);
require_glob("reverse/", false);

@jpbede jpbede changed the title WIP: JS formatter and prettifier PoC WIP: New Feature: JS formatter and prettifier Oct 28, 2020
@tlimoncelli
Copy link
Copy Markdown
Contributor

Wow! I kind of filed that feature request as a joke because I didn't think anyone would implement it. I had no idea that someone had made a js beatifier that could be used as a library! This is super cool! Really made my day!

I ran the POC on Stack Overflow's dnsconfig.js and it worked really well. Only whitespace changes and nothing too radical.

Of course, I had to file a feature request for my favorite rewrite rule (see ditashi/jsbeautifier-go#7).

This would be a great addition to the project!

@jpbede
Copy link
Copy Markdown
Member Author

jpbede commented Oct 28, 2020

Ok cool, then i invest some more time into it and make it merge ready :)

The library seems to be unmaintained, unfortunately.
I can fork it and implement your feature.

@tlimoncelli
Copy link
Copy Markdown
Contributor

Yeah, the fact that it is unmaintained is very concerning to me. I'm actually worried about otto's lack of updates for the same reason.

Maybe it would be better to provide a shell script wrapper that calls an external beautifier with a fixed set of options?

@jpbede
Copy link
Copy Markdown
Member Author

jpbede commented Oct 29, 2020

Hmm yea would be simpler .. But i liked the idea to have it in dnscontrol without dealing with npm or something else.

@jpbede jpbede closed this Feb 20, 2021
@tlimoncelli tlimoncelli reopened this Feb 20, 2021
@tlimoncelli
Copy link
Copy Markdown
Contributor

I'd like to develop a way to accept experimental features like this.

@jpbede jpbede changed the title WIP: New Feature: JS formatter and prettifier New Feature: JS formatter and prettifier Mar 1, 2021
@jpbede jpbede marked this pull request as ready for review March 1, 2021 19:51
@jpbede
Copy link
Copy Markdown
Member Author

jpbede commented Mar 2, 2021

I've now marked this to be merged. I've also added a [BETA] flag to the command description. I've also added a a parameter to write the file directly.

Copy link
Copy Markdown
Contributor

@tlimoncelli tlimoncelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

Comment thread commands/fmt.go Outdated
Comment thread commands/fmt.go Outdated
Comment thread commands/fmt.go Outdated
@jpbede
Copy link
Copy Markdown
Member Author

jpbede commented Mar 2, 2021

Thanks for the review. I've implemented your feedback

@tlimoncelli
Copy link
Copy Markdown
Contributor

I just reformatted stackoverflow's internal dnsconfig.js and it sure does look better! Thanks!

@tlimoncelli tlimoncelli merged commit 37b02b6 into DNSControl:master Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

dnsconfig.js pretty-fier

2 participants