Ability to toggle normalization of whitespace/tickfmt

[anlutro]

I noticed that with 0.12, Kapacitor normalizes whitespace in TICKscripts. It replaces tabs with 4 spaces, and changes this:

stream
    |from('foo').measurement('bar')

Into this:

stream
    |from('foo')
        .measurement('bar')

I'm not necessarily against that, but am writing some code that does diffs of existing scripts compared to new ones. This obviously breaks if Kapacitor says the TICKscript whitespace is different from what it actually is in the source file.

Can the whitespace normalization be disabled?

[nathanielc]

Can the whitespace normalization be disabled?

Not in the current version. I would think that the normalization would help with the diffs. The system being used to format the scripts is available via the command https://github.com/influxdata/kapacitor/tree/master/tick/cmd/tickfmt or the package function https://github.com/influxdata/kapacitor/blob/master/tick/fmt.go#L9

You can use those resources before performing a diff to get consistent accurate results.

[anlutro]

Aha, I'll probably end up using that. I still think a toggle for this behaviour would be a good idea, though I don't know if it'd be problematic code-wise.

[anlutro]

It looks like tickfmt hasn't been added to the Debian package. Should I open a separate issue for that?

[nathanielc]

Yes that's a good idea thanks
On Apr 8, 2016 12:53 AM, "Andreas Lutro" notifications@github.com wrote:

It looks like tickfmt hasn't been added to the Debian package. Should I
open a separate issue for that?

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#426 (comment)

[anlutro]

Was this implemented somewhere?

[nathanielc]

@anlutro No, l closed the issue since tickfmt is now shipped with the packages. I feel like adding the ability to toggle the formatting behavior doesn't add much value. If the formatting is causing issues we should fix the formatting instead of turning it off. Feel free to reopen the issue if you think differently and we can discuss further.

[anlutro]

Right. Like I said, I want to be able to generate diffs - and if those diffs are different from the actual scripts being input (because they have to be normalized by tickfmt), then the diff could be confusing/misleading.

Normally this is fine because I can replace tabs with 4 spaces and it's close enough, but I have seen a few cases where I've had to change multi-line lambdas to a less readable format. You could make the argument that tickfmt should always output the most readable format, but everything is subjective, and I'd hate it if I had to go through all my TICKscripts every time there was a new kapacitor release purely because tickfmt's style has changed.

Another thing I didn't consider checking is, are comments removed when the script is added?

Also, I can't re-open closed issues, only project maintainers (i.e. you) can.

[nathanielc]

@anlutro Some thoughts...

I'd hate it if I had to go through all my TICKscripts every time there was a new kapacitor release purely because tickfmt's style has changed.

Agreed

Another thing I didn't consider checking is, are comments removed when the script is added?

Comment are preserved.

seen a few cases where I've had to change multi-line lambdas to a less readable format.

I would love to see these example if only to improve the format.

[anlutro]

Here's an example:

.warn(lambda:
    "percent_packet_loss" > 0 OR (
        "average_response_ms" > 100 AND
        sigma("average_response_ms") > 10
    )
)

When I run this through my configuration manager, which does a diff of the file against what the Kapacitor API returns for TICKscript, I always get this diff:

-        .warn(lambda: "percent_packet_loss" > 0 OR
-            ("average_response_ms" > 100 AND
-                sigma("average_response_ms") > 10))
+        .warn(lambda:
+            "percent_packet_loss" > 0 OR (
+                "average_response_ms" > 100 AND
+                sigma("average_response_ms") > 10
+            )
+        )

[nathanielc]

@anlutro If we added the toggle as a flag in the API would that be sufficient. Meaning if we always format the script for the output of kapacitor show but if you call the API directly you can turn it off. Would that work for you?

[anlutro]

Oh, I thought the script was normalized when you store it with kapacitor define.

Sure, a query string or something to disable normalization would work as well.

[nathanielc]

Oh, I thought the script was normalized when you store it with kapacitor define.

It is currently but if we are going to toggle it we should change the behavior to store the exact input and on display format. See #480

[anlutro]

Awesome. Thanks for these great tools 👍