Scintilla 3.3.4 Updating

Don HO
2013-08-22
2013-09-21
1 2 > >> (Page 1 of 2)
  • Don HO
    Don HO
    2013-08-22

    notepad++.exe didn't be modified, so just replace scilexer.dll in your npp 6.4.5

    Don

     
  • Neomi
    Neomi
    2013-08-22

    Is this based on patch #467 or did you upgrade independently? I tested a custom build 3.3.4 version for a bit over a week now, no problems found so far. Anyway, switching to your version now.

    Edit:
    Obviously updated independently, I compared the sources to the 3.3.4 snapshot of Scintilla.

    Things I noticed from the sources...

    1. In almost every file the line endings are different to those in the Scintilla repository (\r\n instead of \n). This actually makes it harder than necessary to keep Scintilla up to date later. Is this a choice for any reason or an unwanted automatic conversion somewhere along the line? Can be fixed easily if there are no reasons against it.

    2. I don't know about the previous version 2.2.7, but in 3.3.4 the custom notifications SCN_SCROLLED and SCN_FOLDINGSTATECHANGED are not needed any longer. Using the official notifications SCN_UPDATEUI (SCNotification::updated contains SC_UPDATE_V_SCROLL) and SCN_MODIFIED (SCNotification::modificationType contains SC_MOD_CHANGEFOLD) should do the job as well while reducing the gap in the sources and therefore maintenance effort. Can be done anytime after the update.

     
    Last edit: Neomi 2013-08-22
    • Don HO
      Don HO
      2013-08-22

      Thank you for your feedback.

      For 2 messages processed already by Scintilla, good new, I'll remove them to make easier updating afterward.

      Regarding the patch 467, could you attach your Scintilla + Notepad++ source code here?

      Don

       
      • Neomi
        Neomi
        2013-08-23

        Of course, here is a cleaned up version of my repository. The temporary build files are removed, so you need to execute BuildBoost.bat and scintilla.mak first (but you know that better than anyone else I guess). I took your changes to "scintilla/lexlib/WordList.h" (better than my changes making everything public again) and your versions of the Notepad++ specific lexers. All the line ending conversion to the Scintilla default is present in this version, except for possibly Windows line endings where changes to Scintilla were made.

         
        Attachments
        • Don HO
          Don HO
          2013-08-23

          Thank you Neomi.

          I build your scintilla 334, and I get crash.
          Step to reproduce:
          1. open function list panel
          2. open several cpp files (5-8)
          3. close opened files one by one
          4. crash.

          This problem can be reproduced in the scilexer.dll I providered.
          Any thought about it?

          Don

           
          • Neomi
            Neomi
            2013-08-23

            No idea without looking into it. I will check that, but I won't be at my own computer until monday (visiting family right now), so it can take a while. Sorry.

             
            • Don HO
              Don HO
              2013-08-25

              SciLexer.dll compiled w/o Boost doesn't crash anymore.
              The crash is due to boost's RegEx obviously.

              Sending SCI_RELEASEDOCUMENT to Scitilla (the version with boost) makes it crash in the following line (document.cxx):

              for (std::vector<WatcherWithUserData>::iterator it = watchers.begin(); it != watchers.end(); ++it) {

              I'm looking at it. If anyone has any clue, please let me know.

              Don

               
              • Ok, found it. When the document is deleted, we (in BoostRegexSearch) are notified that it's deleted, so we delete the watcher. Unfortunately, this is done inside the loop, so then the iterator is bust. Simply not calling RemoveWatcher when the document is deleted should do it. I don't know what's changed, i.e. how this ever worked, but I suspect Scintilla is doing more tidying up than it used to. I'll try to check before I send the patch in.

                Patch to follow.

                Dave.

                 
                • Urgh. Scintilla changed sometime in the 3.x branch from a simple array of watchers, with a lenWatchers, to a vector<Watcher>. With the old method, the for loop would have coped, albeit badly, with removing a watcher inside NotifyDeleted, so it wouldn't have crashed, but might have failed to notify another watcher. It seems removing the watcher inside NotifyDeleted was never a good idea :)

                   
                  • Neomi
                    Neomi
                    2013-08-27

                    I tried your suggested change. In BoostRegexSearch::Match::NotifyDeleted() I replaced

                    set(NULL);
                    

                    with a manually inlined version minus the call to RemoveWatcher():

                    _document = NULL;
                    _position = -1;
                    _endPositionForContinuationCheck = _endPosition = -1;
                    _documentModified = false;
                    

                    Well, I can confirm that it works, the crash (I could reproduce it before the change) is gone. The change seems to be a clean solution too. Since the list of watchers is part of the document and deleted with it anyway, there are no dead entries left.

                     
                  • Don HO
                    Don HO
                    2013-08-27

                    Thank you Dave for the patch.
                    I confirm you it fixes the crash.

                    Don

                     
  • THEVENOT Guy
    THEVENOT Guy
    2013-08-25

    Hi Don,

    Thank you for the update. Just a small info :

    It's not very important, but, when I dezipped the archive, I noticed that information of version of the new Scintilla.dll have not been updated ( still 2.2.7 version, instead of 3.3.4 ! )

    Cheers,

    guy038

     
    Last edit: THEVENOT Guy 2013-08-25
    • Don HO
      Don HO
      2013-08-25

      Thank you Guy!
      It's fixed now.

      Don

       
  • FLS
    FLS
    2013-08-29

    Dear Don
    Great to see, that Scintilla within Notepad++ is upgraded to version 3.3.4. I have just ‘downloaded’ SVN 1104. Please let me put here some ideas:

    1.) Keeping the original Scintilla files as much as possible without modifications (even the line endings (\n)) should make it easier to keep the Notepad++ Scintilla in line with the actual Scintilla version. An approach for that is outlined in patch #467 “Upgrade/Merge of NP++-Scintilla 2.2.7 to version 3.3.0”.

    2.) With more frequently Scintilla updates it would be possible to correct some Scintilla related issues within Notepad++ directly in Scintilla (via bug report and patch) and get it back in the main stream of Scintilla. A good example for that is the work from Neomi, who provides a patch to Scintilla on account of an annoying behaviour within Notepad++:

    Bugfix [#1512] Fix bug with horizontal caret position when margin changed, which was implemented within Scintilla on 08.08.13.

    3.) Maintainability of the Notepad++ Scintilla version can be improved by reducing Notepad++ specific changes within Scintilla files to a minimum and if possible, putting them into extra files or mark those explicitly as Notepad++ changes like:

        //-- NppExtension: - <what was changed and why>“
    

    (For the version 3.3.0 this is done at patch #467 “Upgrade/Merge of NP++-Scintilla 2.2.7 to version 3.3.0”)

    4.) It would be very welcome to adapt the additional Notepad++ lexers (LexObjC, LexSearchResult, LexUser) and BoostRegEx to the current Scintilla code in order to avoid Notepad++ specific adaptations/patches within the Scintilla files to get those lexers running.

    5.) For building the Notepad++ specific SciLexer.dll the Scintilla subdirectories cocoa, gtk, qt, scripts and maybe test are not needed (not used). For more clearness and simplicity within Notepad++, these directories should be deleted from Notepad++ repository.

    6.) The SciLexer.dll compiled with Notepad++ is, because of the adaptations, not the same as the original Scintilla-DLL. Therefore, the version information should report that. For example the following changes could be made in the ScintRes.rc file for Notepad++:

            #define VERSION_SCINTILLA "3.3.4-NP++"
            #define VERSION_WORDS 3, 3, 4, 1
            VALUE "CompanyName", "Neil Hodgson neilh@scintilla.org & Notepad++\0"
    

    Finally hoping the good work with Notepad++ can go straight on.

    FLS

     
    • Don HO
      Don HO
      2013-08-30

      Hi FLS,

      1.) Keeping the original Scintilla files as much as possible without modifications (even the line endings (\n)) should make it easier to keep the Notepad++ Scintilla in line with the actual Scintilla version.

      Agree.
      However, I download the latest official release from:
      http://prdownloads.sourceforge.net/scintilla/scintilla334.zip?download
      That explains why we got the different EOL.

      With more frequently Scintilla updates it would be possible to correct some Scintilla related issues within Notepad++ directly in Scintilla (via bug report and patch) and get it back in the main stream of Scintilla. A good example for that is the work from Neomi, who provides a patch to Scintilla on account of an annoying behaviour within Notepad++...

      I will do my best to keep it updated.

      Maintainability of the Notepad++ Scintilla version can be improved by reducing Notepad++ specific changes within Scintilla files to a minimum and if possible, putting them into extra files or mark those explicitly as Notepad++ changes like:
      “//-- NppExtension: - <what was="" changed="" and="" why="">“

      This part, however, it's not necessary IMO.
      In the latest revision, I added trunk\PowerEditor\scintilla.original.forUpdating\scintilla.original.forUpdating.7z
      scintilla.original.forUpdating.7z contain the original compact scintilla package. In the latest revision it's scintilla's 3.3.4 official release.

      So next updating time (say update to 3.6), we need
      1. Download 3.6 official release.
      2. Copy original v3.6 for afterward.
      3. Integrate the diff between scintilla in repo and scintilla.original.forUpdating.7 into v3.6
      4. Compact the original v3.6 into scintilla.original.forUpdating.7z to replace the old one.
      5. Remove deleted fils/folder from repo.
      6. Put merged 3.6 into repo.

      This procedure allows us to remove deleted files/folders in the newest version from the repo.

      It would be very welcome to adapt the additional Notepad++ lexers (LexObjC, LexSearchResult, LexUser) and BoostRegEx to the current Scintilla code in order to avoid Notepad++ specific adaptations/patches within the Scintilla files to get those lexers running.

      I'm not against it but I don't think Neil will accept them. With several failed experiences to submit the patches, I am not willing to deal with him. So you are on your own if you want to do it.

      For building the Notepad++ specific SciLexer.dll the Scintilla subdirectories cocoa, gtk, qt, scripts and maybe test are not needed (not used). For more clearness and simplicity within Notepad++, these directories should be deleted from Notepad++ repository.

      They are useless but why bother to delete them? Furthermore, keeping them will make next updating more easier since you use the (almost) same source to get the diff.

      The SciLexer.dll compiled with Notepad++ is, because of the adaptations, not the same as the original Scintilla-DLL. Therefore, the version information should report that. For example the following changes could be made in the ScintRes.rc file for Notepad++

      Indeed. I would say :
      define VERSION_SCINTILLA "3.3.4 for Notepad++"

      It'll be applied in the next release.

      Thank you for your suggestion.

      Don

       
      Last edit: Don HO 2013-08-30
      • FLS
        FLS
        2013-09-01

        Hi Don

        Thank you for your comprehensive answer.

        1.) When I upgraded from Scintilla 2.2.7 to 3.3.0 and (in parallel to Neomi) to 3.3.4, I compared the original 2.2.7 version with that of Notepad++. However, I found it very difficult to decide, if the changes made in the Notepad++ version are still valid and are working together with the upgrades made in the original Scintilla version. Furthermore, I had to figure out, for which purpose they were made. That was the reason to suggest marking of the Notepad++ changes and giving the reason for the adaptation like:
        “//-- NppExtension: - what was changed and why“

        2.) About procedure to incorporate Scintilla upgrades into the Notepad++ version

        3. Integrate the diff between scintilla in repo and scintilla.original.forUpdating.7 into v3.6
        

        In which way, with which tools do you do this?
        Would you also have to compare also the "unnecessary" files in subdirectories cocoa, gtk, qt, scripts and test?

        My procedure uses the original Scintilla Mercurial repository at http://hg.code.sf.net/p/scintilla/code reachable via the Scintilla project page https://sourceforge.net/projects/scintilla/:

        1. Install a Mercurial client (e.g. TortoiseHg)

        2. With TortoiseHg, clone the Scintilla repository to a local working copy and update to the version, on which the current Notepad++ version was based (e.g. Scintilla version 2.2.7).

        3. Copy (overwriting) the current Notepad++-Scintilla files (as of SVN revision 1018) into that Mercurial working copy.
          You have to commit that changes within the local working copy. This is a little bit differen to Subversion, because the "cloned" local working copy is indeed itself a Mercurial repository.

        4. Use TortioseHg to update (Mercurial command is "merge with local") those files with the current Scintilla version (or any other specific Scintilla revision) and solve some conflicts manually (mainly for the Notepad++-Scintilla modifications).
          At that point I think it would be helpful to have the Notepad++ modifications with the reason for the adaptation marked.
          TortoiseHg works nearly as Subversion. It automatically merges the changes in the Mercurial repository with the changes made in the local working copy and supports solving conflicts with TortoiseMerge tool (or any other diff-tool).

        5. Try to build Scintilla in the original Scintilla working copy to solve first compiling issues. All changes with respect to the original Scintilla are handled using TortoiseHg within this local working copy.

        6. Copy the necessary merged files (omitting unnecessary Scintilla subdirectories) into the Notepad++ working copy.

        7. Build the SciLexer.dll for Notepad++.

        8. Compile Notepad++ and run / test it.

        9. Commit the changes into Notepad++ Subversion repository.

        10. For comparison and some tests, I have a second clone of the Scintilla repository on my disk, where no changes are made. So that is the reference.
          Your idea is very good to put a 7z-file of the original Scintilla files as reference into the Notepad++ repository.

        However, such work is done best with the approach of one's own.

        Regards
        FLS

         
        • Don HO
          Don HO
          2013-09-19

          I use winmerge to compare the difference then merge manually into Scintilla newest release.

          I have no intention to deal with Mercurial, the procedure I provide above seems quite straight forward to me.

          Don

           
          • Neomi
            Neomi
            2013-09-19

            I don't really understand what you have against Mercurial, your way seems to be a lot more tedious and error prone. May I suggest a third way?

            As much as I love WinMerge (it is a great tool and I use it quite often myself), it is the wrong tool for this kind of job. The right tool is a 3-way merge tool. These tools don't just compare two files with each other, they compare both to a common base. They therefore have a lot more context and reduce the manual merging to the few real conflicts.

            The tool I use is KDiff3 (part of my TortoiseHg installation, but can be used by itself):
            http://kdiff3.sourceforge.net/

            Please give it a try, you will most likely end up thinking "how could I ever do it without it". It doesn't look as polished as WinMerge, but it makes merging easier. I would have suggested P4Merge, but it sadly cannot operate on folders.

            I attached a screenshot of an example. I set up the folders Sci334 as the base, Npp334 as the first modification, Sci335 as the second modification and Npp335 as the merge destination.

             
            Attachments
  • Don HO
    Don HO
    2013-08-30

    Neomi,

    I just compared your source (scintilla & notepad++) with mine, I didn't see any change in your code regarding using of "SCN_UPDATEUI (SCNotification::updated contains SC_UPDATE_V_SCROLL) and SCN_MODIFIED (SCNotification::modificationType contains SC_MOD_CHANGEFOLD)".

    Regarding the different EOL, I download the latest official release from:
    http://prdownloads.sourceforge.net/scintilla/scintilla334.zip?download

    I suppose that you get directly the latest SVN revision (3.3.4.1). That explains.

    Don

     
    • Neomi
      Neomi
      2013-08-30

      Hi Don,

      the replacements of the custom notifications by the ones existing in the original sources is something I didn't implement. I just noticed that they are there while learning the Scintilla interface, so I mentioned them. But I can make the changes and give you an updated source package this weekend (most likely).

      And yes, I didn't use the download package. I used TortoiseHg since Scintilla is in a Mercurial repository and used the unmodified original. That made it easier to make the patches I submitted to Scintilla (will be included in 3.3.5 which should be out next week). In fact, I have multiple copies of the repository. One where I make patches that go back into Scintilla and one which contains the Notepad++ modifications for almost automatic updating. The official version number is still 3.3.4, I took the .1 and other changes from the patch FLS submitted. He actually did most of the work with 3.3.0, my update was quite small in comparison.

       
  • Neomi
    Neomi
    2013-08-31

    As promised, here is my update, both as a patch file relative to revision 1104 and as a cleaned up copy of my working directory, whichever is easier for you to use.

    I could get rid of SCN_SCROLLED without problems, but SCN_FOLDINGSTATECHANGED was a bit different to what I expected. It is not send from a modification to Scintilla, but from inside Notepad++ itself in a specific case. Also SCN_MODIFIED with SC_MOD_CHANGEFOLD is already processed, but in a different way. Since I don't want to break any existing functionality and I don't have a complete understanding of the inner workings, SCN_FOLDINGSTATECHANGED is still in there for now.

    Patch #515 is already included in the attached files since you accepted it for the next release.

    In addition to that, I upgraded further to Scintilla 3.3.5 which has just been released. There were no conflicts while updating, it contains a few bug fixes and improvements.

     
    • Don HO
      Don HO
      2013-09-02

      Your modif has been submitted in SVN (rev.1106).
      I added only the removal SCN_SCROLLED part, Scintilla will be upgraded afterward.
      I didn't add "_smartHighlighter.highlightView(notifyView);" neither. Smart highlighter highlight all in the document in this version of scintilla, and I don't see any performance issue regarding this.

      Thank you for your patch.

      Don

       
  • GerdB
    GerdB
    2013-09-18

    Good to see that NPP is going to catch up with Scintilla Versions. Are there any estimates when this will be in main trunk?

    regards
    gerd

     
    • dail8859
      dail8859
      2013-09-18

      It already is according to this commit 3 weeks ago. :)

       
1 2 > >> (Page 1 of 2)