#981 AutoComp: allow non-alphabetical sort

Completed
closed
scintilla (101)
4
2013-04-12
2013-03-02
Alpha
No

Allow the auto-complete listing to work when given a list that is not alphabetically sorted (ex. sorted by priority).

1 Attachments

Discussion

<< < 1 2 3 4 > >> (Page 2 of 4)
  • Neil Hodgson

    Neil Hodgson - 2013-03-06

    g++ -DNDEBUG -Os -Wall -Wno-missing-braces -Wno-char-subscripts -pedantic -I ../include -I ../src -I../lexlib -fno-rtti -c ../src/AutoComplete.cxx
    ../src/AutoComplete.cxx: In member function 'void AutoComplete::SetList(const char*)':
    ../src/AutoComplete.cxx:164:25: warning: ISO C++ forbids variable length array 'sortedList' [-Wvla]

    g++ --version
    g++ (GCC) 4.7.2

     
  • Alpha

    Alpha - 2013-03-06

    Switched to char array to std::string.

     
  • Neil Hodgson

    Neil Hodgson - 2013-03-06

    An explanation should cover all the effects of the choice in full sentences, not as a short snippet. This is for people that who are coming to this new.

     
  • Alpha

    Alpha - 2013-03-07

    Are these comments descriptive enough?

    I also switched the constructor to autoSort(0) so the default behaviour will remain the same as previously.

     
    • Neil Hodgson

      Neil Hodgson - 2013-03-08

      OK. There's a large feature landing soon so this won't be committed until after that. May be a week or two.

       
  • Alpha

    Alpha - 2013-03-08

    Understood.

     
  • Neil Hodgson

    Neil Hodgson - 2013-03-16

    The code assumes that there is a terminating separator when there may not be. For example, the call may be SCI_AUTOCSHOW(1, "aardvark\nant"), not SCI_AUTOCSHOW(1, "aardvark\nant\n"). This often results in an extra garbage value in the list. It also means that the code that builds the sorted string may merge two values so "ant\naardvark" will produce a list containing an item "aardvarkant".

    If possible the "Check for a logically earlier match" should be moved out of the binary search. Yes, I know there is a similar case for SC_CASEINSENSITIVEBEHAVIOUR_RESPECTCASE. At some point I'd like to replace the binary search code with a call to std::binary_search.

    A patch is attached with the changes to other files required for this. Different orders have symbolic names that should be used instead of numeric literals. SC_ORDER_PERFORMSORT and SC_ORDER_UNSORTED have swapped values since it makes more sense as a narrative. SC_ORDER_UNSORTED is a little purposeless: something like SC_ORDER_PRIORITY may be better.

     
  • Alpha

    Alpha - 2013-03-17

    Fixed the terminating separator issue, incorporated the changes from your patch, and renamed SC_ORDER_UNSORTED to SC_ORDER_CUSTOM. I did not move the earlier match check because it would, in my opinion, be less clear.

     
  • Neil Hodgson

    Neil Hodgson - 2013-03-17

    The separator issue still causes problems: its not just that it still displays a garbage value but it also reads into any garbage after the argument. The \0 at the end of the argument is reached by the third ++i inside Sorter::Sorter (line 119) and then the for loop's ++i moves i into post \0 garbage.
    alternate text

    The problem with the extra match code is that it is inside the binary search loop. This raises questions like: Can it be executed more than once? What are the effects of executing it more than once? Can it prevent the binary search from terminating? Moving it outside the binary search makes it much easier to analyze its behaviour.

     
    Last edit: Neil Hodgson 2013-03-17
  • Alpha

    Alpha - 2013-03-19

    I see. Although I have been unable to reproduce this, I believe this patch should resolve it.

     
<< < 1 2 3 4 > >> (Page 2 of 4)

Log in to post a comment.