dnscontrol icon indicating copy to clipboard operation
dnscontrol copied to clipboard

trailing comma creates non-descript error

Open gotjoshua opened this issue 1 year ago • 11 comments

Describe the bug when using format=js if there is a trailing comma, an error is thrown. Can happen when the commented //NAMESERVER line is after a normal record with a trailing comma

executing dnsconfig.js: (anonymous): Line 52:1 Unexpected token ) (and 1 more errors)

To Reproduce

  1. dnsconfig.js:
D("foobar.me", REG_CHANGEME,
	DnsProvider(DSP_WHATEVER),
	CNAME("foo", "bar.xyz."),
	//NAMESERVER("bar.")
)
  1. dnscontrol preview

Expected behavior Tolerate trailing commas (especially if they are produced by get-zones

DNS Provider

  • CLOUDNS (but not relevant)

gotjoshua avatar Feb 23 '24 21:02 gotjoshua

Maybe you should try -format=djs?

   --format=js        dnsconfig.js format (not perfect, just a decent first draft)
   --format=djs       js with disco commas (leading commas)

imlonghao avatar Feb 24 '24 10:02 imlonghao

Another trick is to close your D() with END):

D("foo.com", ...
    A(...),
    A(...),
    A(...),
END)

END is just an alias for {}, which is ignored by DNSControl. However it makes a comma on the previous line required, like all other lines.

I think END should be the default. I'm fixing this here: https://github.com/StackExchange/dnscontrol/pull/2849

tlimoncelli avatar Feb 24 '24 14:02 tlimoncelli

Another trick is to close your D() with END):

D("foo.com", ...
    A(...),
    A(...),
    A(...),
END)

END is just an alias for {}, which is ignored by DNSControl. However it makes a comma on the previous line required, like all other lines.

Cool! I didn't know this and it was always a small frustration. Maybe something to document as tips and tricks?

cafferata avatar Feb 24 '24 18:02 cafferata

Maybe you should try -format=djs?

yes, i did that and it works, but i find it rather ugly. and still the bug is legit in js mode.

gotjoshua avatar Feb 28 '24 09:02 gotjoshua

I think END should be the default. I'm fixing this here: #2849

thanks for the fix, will it also work if a commented line is just above the END ?

gotjoshua avatar Feb 28 '24 09:02 gotjoshua

Hmmm... a comment the line before the END should work, assuming the line above it ends in a comma.

   A(),
   // comment
   END);

tlimoncelli avatar Feb 28 '24 12:02 tlimoncelli

I think this issue should still be open: the issue is still present and is rather frustrating when the trailing comma is added by an editor's on-save formatting. Both prettier and deno fmt will split long lines adding trailing commas by default, including after END, thereby negating its usefulness [for users where a formatter will be applied].

Fortunately it seems as though a possible fix has been merged in otto, so after its next release and a dependency update here, this won't be an issue any more.

I worked around it in the meantime by reconfiguring prettier:

/**
 * @see https://prettier.io/docs/en/configuration.html
 * @type {import("prettier").Config}
 */
const config = {
  trailingComma: 'es5',
  tabWidth: 2,
  semi: false,
  singleQuote: true,
}

export default config

alanpearce avatar Jun 04 '24 17:06 alanpearce

Hi there!

I've reopened the bug as requested.

Can you give an example of how END is affected? Maybe a "before and after" would help.

tlimoncelli avatar Jun 04 '24 17:06 tlimoncelli

source: commands/test_data/ds.com.js

before:

var DSP_BIND = NewDnsProvider("bind", "BIND");
var REG_CHANGEME = NewRegistrar("none");

D("ds.com", REG_CHANGEME,
	DnsProvider(DSP_BIND),
	//SOA("@", "ns3.serverfault.com.", "sysadmin.stackoverflow.com.", 2020022300, 3600, 600, 604800, 1440),
	DS("geo", 14480, 13, 2, "BB1C4B615CDED2B34347CF23710471934D972F1E34F53B54ED8D5F786202C73B"),
END);

after deno fmt or prettier

var DSP_BIND = NewDnsProvider("bind", "BIND");
var REG_CHANGEME = NewRegistrar("none");

D(
  "ds.com",
  REG_CHANGEME,
  DnsProvider(DSP_BIND),
  //SOA("@", "ns3.serverfault.com.", "sysadmin.stackoverflow.com.", 2020022300, 3600, 600, 604800, 1
440),
  DS(
    "geo",
    14480,
    13,
    2,
    "BB1C4B615CDED2B34347CF23710471934D972F1E34F53B54ED8D5F786202C73B",
  ),
  END,
);

alanpearce avatar Jun 04 '24 18:06 alanpearce

Oh! I see!

The editor is adding a , after END -- the result being END,); which fmt rewrites as END,\n);

Yeah, that's not good.

Which editor does this? Can that feature be disabled?

tlimoncelli avatar Jun 04 '24 18:06 tlimoncelli

It's not the editor itself (in my case Emacs), it's any editor or language server that uses prettier or deno (possibly others?) to format code automatically on save.

Format on save can be disabled, but I think the better workaround is to reconfigure prettier so that it understands that otto can only parse es5-style trailing commas (as of 0.4.0), which is the workaround in my previous comment and even this repository.

Deno doesn't appear to have a setting for trailing commas, because it is both an interpreter and formatter (so the formatter knows what the interpreter supports) and new enough that trailing commas were always supported everywhere. Fortunately, prettier is the common choice.

alanpearce avatar Jun 04 '24 19:06 alanpearce

The proper way to format dnsconfig.js files is by running "dnscontrol fmt". I recommend updating your editor to use that.

Otherwise, since the original bug has been remediated, I'm closing this issue. Please feel free to re-open this issue or start a new one.

tlimoncelli avatar Sep 09 '24 22:09 tlimoncelli