Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.1
    • Component/s: None
    • Labels:
      None

      Description

      Improve the uniformity and scale of solr indenting (solr only supported 7 levels of indenting previously)

      1. SOLR-1933.patch
        2 kB
        Yonik Seeley

        Activity

        Hide
        Grant Ingersoll added a comment -

        Bulk close for 3.1.0 release

        Show
        Grant Ingersoll added a comment - Bulk close for 3.1.0 release
        Hide
        Hoss Man added a comment -

        Didn't seem useful beyond that (an indent of 40 chars)

        i'm mainly just wondering about having an odd indent break in hte middle of a data structure

        having a fixed limit lets one optimize the indent by implementing it with a singe write out of a char array

        that was the part i wasn't thinking through – using a single write. totaly makes sense.

        Show
        Hoss Man added a comment - Didn't seem useful beyond that (an indent of 40 chars) i'm mainly just wondering about having an odd indent break in hte middle of a data structure having a fixed limit lets one optimize the indent by implementing it with a singe write out of a char array that was the part i wasn't thinking through – using a single write. totaly makes sense.
        Hide
        Yonik Seeley added a comment -

        why stop at 20?

        Didn't seem useful beyond that (an indent of 40 chars), and having a fixed limit lets one optimize the indent by implementing it with a singe write out of a char array. It would be easy to up that to 80 chars (40 levels of indents)... anything in that range seems like it will be pretty unreadable anyway, with or without additional indenting.

        Show
        Yonik Seeley added a comment - why stop at 20? Didn't seem useful beyond that (an indent of 40 chars), and having a fixed limit lets one optimize the indent by implementing it with a singe write out of a char array. It would be easy to up that to 80 chars (40 levels of indents)... anything in that range seems like it will be pretty unreadable anyway, with or without additional indenting.
        Hide
        Hoss Man added a comment -

        improve the indenting at the cost of bandwidth.

        +1 Yeah!!!!! ... no more tabs from solr!!!!

        2 space indents up to 20 levels.

        why stop at 20?

        if they want indenting, it's for human consumption - if it's for human consumption, and they have data nested deeper then 20 levels, we might as well go all the way (because data that complicated is just going to be more complicated if it stops indenting consistently.

        I don't think we should make it configurable (beyond turning on/off)

        +1

        Show
        Hoss Man added a comment - improve the indenting at the cost of bandwidth. +1 Yeah!!!!! ... no more tabs from solr!!!! 2 space indents up to 20 levels. why stop at 20? if they want indenting, it's for human consumption - if it's for human consumption, and they have data nested deeper then 20 levels, we might as well go all the way (because data that complicated is just going to be more complicated if it stops indenting consistently. I don't think we should make it configurable (beyond turning on/off) +1
        Hide
        Yonik Seeley added a comment -

        Here's a quick patch that supports 2 space indents up to 20 levels.

        And before anyone asks... I don't think we should make it configurable (beyond turning on/off). It would be extra complexity + params to check for every request, with very very little to gain.

        Any concerns? I don't believe anyone would be using indent=on for high volume queries in production.

        Show
        Yonik Seeley added a comment - Here's a quick patch that supports 2 space indents up to 20 levels. And before anyone asks... I don't think we should make it configurable (beyond turning on/off). It would be extra complexity + params to check for every request, with very very little to gain. Any concerns? I don't believe anyone would be using indent=on for high volume queries in production.
        Hide
        Yonik Seeley added a comment -

        Background: the very first versions of Solr always indented... and hence only supported indenting up to 7 levels, and was very frugal in it's use of bandwidth (most indenting levels consisted of just 2 characters in total, not including the newline). It did this through a mix of tabs and spaces, which lead to varying width for each indentation level.

         private static final String[] indentArr = new String[] {
            "\n",
            "\n ",
            "\n  ",
            "\n\t",
            "\n\t ",
            "\n\t  ",  // could skip this one (the only 3 char seq)
            "\n\t\t" };
        

        Given that there is now an intent=true/false parameter that defaults to false (this has been there since Solr was open-sourced), we should perhaps improve the indenting at the cost of bandwidth.

        Show
        Yonik Seeley added a comment - Background: the very first versions of Solr always indented... and hence only supported indenting up to 7 levels, and was very frugal in it's use of bandwidth (most indenting levels consisted of just 2 characters in total, not including the newline). It did this through a mix of tabs and spaces, which lead to varying width for each indentation level. private static final String [] indentArr = new String [] { "\n" , "\n " , "\n " , "\n\t" , "\n\t " , "\n\t " , // could skip this one (the only 3 char seq) "\n\t\t" }; Given that there is now an intent=true/false parameter that defaults to false (this has been there since Solr was open-sourced), we should perhaps improve the indenting at the cost of bandwidth.

          People

          • Assignee:
            Unassigned
            Reporter:
            Yonik Seeley
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development