Skip to content

Conversation

@knz
Copy link
Contributor

@knz knz commented Apr 12, 2019

This PR removes the ability to customize keyword casing, simplify
expressions and change the alignment mode.

Instead it proposes just two modes:

  • the default "compact" mode uses "full" alignment and simplificaiton
  • the optional "expanded" mode uses no alignment and does not simplify
    expressions

Example, "compact":

  SELECT count(*) AS count, winner, counter * 60 * 5 AS counter
    FROM (
			SELECT winner, round(length / (60 * 5)) AS counter
			  FROM players
			 WHERE build = $1 AND (hero = $2 OR region = $3)
         )
GROUP BY winner, counter;

Example, "expanded":

SELECT
	count(*) AS count, winner, counter * (60 * 5) AS counter
FROM
	(
		SELECT
			winner, round(length / (60 * 5)) AS counter
		FROM
			players
		WHERE
			build = $1 AND (hero = $2 OR region = $3)
	)
GROUP BY
	winner, counter;

This change is Reviewable

This PR removes the ability to customize keyword casing, simplify
expressions and change the alignment mode.

Instead it proposes just two modes:

- the default "compact" mode uses "full" alignment and simplificaiton
- the optional "expanded" mode uses no alignment and does not simplify
  expressions

Example, "compact":

```
  SELECT count(*) AS count, winner, counter * 60 * 5 AS counter
    FROM (
			SELECT winner, round(length / (60 * 5)) AS counter
			  FROM players
			 WHERE build = $1 AND (hero = $2 OR region = $3)
         )
GROUP BY winner, counter;
```

Example, "expanded":

```
SELECT
	count(*) AS count, winner, counter * (60 * 5) AS counter
FROM
	(
		SELECT
			winner, round(length / (60 * 5)) AS counter
		FROM
			players
		WHERE
			build = $1 AND (hero = $2 OR region = $3)
	)
GROUP BY
	winner, counter;
```
@madelynnblue
Copy link
Owner

I think I understand the goals if this PR based on our hangout. It would allow us to support various use cases where users can choose a bit between readability and vertical length. I'm not yet convinced this is a knob we want to expose.

I fairly strongly think leaving the simplify option as an explicit choice is necessary because some users will not want us to change the text of their query at all, which is reasonable. For example leaving unnecessary parens can be very useful to disambiguate to human readers the order of operations since sometimes we don't have the precedence table memorized. I'm not convinced that worrying about vertical compactness deserves this top level of importance.

I'm much more worried about making most people's use of this tool great, and not worrying too much about users who don't fit in completely into that target. Like for example books or other page-break mediums. There are definitely people who care about that stuff, but I think it's better to make the experience for the relatively larger number of other users better and simpler than providing a tool that works for these other smaller groups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants