Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect Javascript switch case indentation #76

Closed
JaapJoris opened this issue Feb 10, 2023 · 1 comment · Fixed by #77
Closed

Incorrect Javascript switch case indentation #76

JaapJoris opened this issue Feb 10, 2023 · 1 comment · Fixed by #77
Labels
bug Something isn't working

Comments

@JaapJoris
Copy link
Member

There are conflicting opinions about the correct way to indent the switch statement. On the one hand, there is the "old-fashioned" way, recommended by the Crockford Conventions and ESLint:

switch (foo) {
case bar:
    if (baz) {
        switch (foo) {
        case bar:
            if (baz) {
                xizzy();
            }
        }
    }
}

On the other hand, there is the "modern" way, recommended by MDN, and also preferred by most people I asked:

switch (foo) {
    case bar:
        if (baz) {
            switch (foo) {
                case bar:
                    if (baz) {
                        xizzy();
                    }
             }
        }
}

While this example illustrates that the "old-fashioned" way may not be so bad after all, DjHTML currently attempts to indent the "modern" way, and since most people prefer that we should keep it that way. However, it fails spectacularly on this particular example:

switch (foo) {
    case bar:
        if (baz) {
            switch (foo) {
            case bar:
                if (baz) {
                    xizzy();
                }
            }
}
}

I'm hoping to come up with a more generic solution involving offsets that can also solve #50, #59, and #74. Stay tuned!

@JaapJoris
Copy link
Member Author

By the way, my personal preference would actually be this non-standard way:

switch (foo) {
    case bar:
    if (baz) {
        switch (foo) {
            case bar:
            if (baz) {
                xizzy();
            }
        }
    }
}

This makes sense because the case statements are actually a special case of labels, and labels should never be indented. Oh well.

@JaapJoris JaapJoris self-assigned this Feb 10, 2023
JaapJoris added a commit that referenced this issue Feb 13, 2023
For this release, a substantial part the codebase has been refactored.
Here is a non-exhaustive summary of all the things that have changed:

- Support for Python 3.6 and 3.7 has been dropped
- A Django middleware class to indent HTML responses has been added
- Ignore both opening and closing comment tags
- New handling off both relative and absolute offsets
- Return correct `repr()` strings with the `--debug` option
- New token class "OpenDouble" and a revised indentation algorithm
- Additional `line` argument to `create_token()` methods
- Changed return value of `create_token()` methods
- Changed constructor arguments of Token and Line classes
- Refactored all `create_token()` methods to get rid of return statements
- Simplified test suite runner that reindents the expected output
- Don't indent the contents of template variables (closes #75)
- Improved handling of Django tags inside HTML elements
- Improved JavaScript `case` indentation (closes #76)
- Improved JavaScript method chaining (closes #59)
- Improved CSS multiline statements (closes #74)
- New multiline HTML element indentation (closes #50)
- New multiline HTML attribute value indentation
- Extensive test coverage with lots of edge cases
JaapJoris added a commit that referenced this issue Feb 13, 2023
For this release, a substantial part the codebase has been refactored.
Here is a non-exhaustive summary of all the things that have changed:

- Support for Python 3.6 and 3.7 has been dropped
- A Django middleware class to indent HTML responses has been added
- Ignore both opening and closing comment tags
- New handling off both relative and absolute offsets
- Return correct `repr()` strings with the `--debug` option
- New token class "OpenDouble" and a revised indentation algorithm
- Additional `line` argument to `create_token()` methods
- Changed return value of `create_token()` methods
- Changed constructor arguments of Token and Line classes
- Refactored all `create_token()` methods to get rid of return statements
- Simplified test suite runner that reindents the expected output
- Don't indent the contents of template variables (closes #75)
- Improved handling of Django tags inside HTML elements
- Improved JavaScript `case` indentation (closes #76)
- Improved JavaScript method chaining (closes #59)
- Improved CSS multiline statements (closes #74)
- New multiline HTML element indentation (closes #50)
- New multiline HTML attribute value indentation
- Extensive test coverage with lots of edge cases
JaapJoris added a commit that referenced this issue Feb 13, 2023
For this release, a substantial part the codebase has been refactored.
Here is a non-exhaustive summary of all the things that have changed:

- Support for Python 3.6 and 3.7 has been dropped
- A Django middleware class to indent HTML responses has been added
- Ignore both opening and closing comment tags
- New handling off both relative and absolute offsets
- Return correct `repr()` strings with the `--debug` option
- New token class "OpenDouble" and a revised indentation algorithm
- Additional `line` argument to `create_token()` methods
- Changed return value of `create_token()` methods
- Changed constructor arguments of Token and Line classes
- Refactored all `create_token()` methods to get rid of return statements
- Simplified test suite runner that reindents the expected output
- Don't indent the contents of template variables (closes #75)
- Improved handling of Django tags inside HTML elements
- Improved JavaScript `case` indentation (closes #76)
- Improved JavaScript method chaining (closes #59)
- Improved CSS multiline statements (closes #74)
- New multiline HTML element indentation (closes #50)
- New multiline HTML attribute value indentation
- Extensive test coverage with lots of edge cases
@JaapJoris JaapJoris added the bug Something isn't working label Feb 13, 2023
@JaapJoris JaapJoris removed their assignment Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant