Menu

#1344 Add a pointer/LPARAM to each line, and a notification when a line is removed.

Committed
closed
nobody
5
2020-04-27
2020-04-01
Iain Clarke
No

The motivation is simple - I am writing a domain specific IDE, and am using Markers (SCI_MARKERADD etc) to mark breakpoints. This is great as far as it goes - if you add code/text in between, the markers move up and down to different line numbers along with the code they're associated with.

But I want to add extra information to those breakpoints (ie, condition), and there's nowhere to store it. I could use some external place, with a mapping from line number to metadata, but that would be very fragile.

Ideally, I'd want to attach it to the text line itself. This ability is all but in Scintilla. I could use Annotations, put information in them, and just make annotations invisible. But I would also like to use Annotations later for their given purpose later on.

So, I would need something like:
SCI_ADDLINEMETADATA (nLine, void *pLineMetadata).

That I think is well within my ability to add, and if it was that simple, I'd have included a patch with this post. But we don't want leaks, so there would need to be some notification when a line (either always, or only if pLineMetadata != 0) is deleted, so the host application can delete the data, or some other clever thing.

Or would something like:
struct Metadata
{
uint nSize;
byte pData
}
SCI_ADDLINEMETADATA (nLine, struct Metadata
pLineMetadata);
be better, where the metadata is copied into a blob stored with each line, and the original just passed in is the hosts problem?

Then scintilla can tidy the line-metadata-blob when a line is removed, in the same way it tidies markers and annotations?

I am happy to do the work, but I'd welcome some opinion on what would best fit with Scintilla's programming style/philosophy.

Related

Feature Requests: #1344

Discussion

  • Iain Clarke

    Iain Clarke - 2020-04-01

    (There would also need to be a SCI_GETLINEMETADATA message, but that's straightforward.

     
  • Neil Hodgson

    Neil Hodgson - 2020-04-01

    The current way to implement this is to use the handle returned from SCI_MARKERADD as a key into a map.

    There are some limitations with marker handles. SCI_MARKERLINEFROMHANDLE and SCI_MARKERDELETEHANDLE perform linear searches over an array of lines but it is workable in many cases.

    Almost all deletions preserve the set of markers, just moving/merging them onto surviving lines, so a marker handle deletion notification would not be very useful.

     
    • Iain Clarke

      Iain Clarke - 2020-04-02

      That was looking very promising... but I ended up with:

      Line 0, marker
      Line 1, no marker
      Line 2, marker.
      Handle map: 1->0, 2->2

      Delete Line 1, you get:
      Line 0, marker
      Line 1, marker.
      Handle map: 1->0, 2->1
      Great so far!

      Delete line 2 again, you get:
      Line 0, marker
      Handle map: 1->0, 2->0
      Uhoh... 1 marker, 2 handles to it.

      If that's by design, then this will be more than a bit complex.

      If it's a bug, then I will dig in to fix it.

       
      • Iain Clarke

        Iain Clarke - 2020-04-02

        Though this is looking a bit more like a bug report now, than a feature request...

         
      • Neil Hodgson

        Neil Hodgson - 2020-04-03

        That is actually deliberate. Scintilla has no way of knowing the semantics of the markers so preserves all of them even if there are 2 on a single line with the same visual. Perhaps they are breakpoints and one is a standard breakpoint and the other is a tracepoint or conditional breakpoint (with condition expression data stored by the application for that handle). Scintilla has no way of knowing what the correct thing to do is. The application may then choose to remove one or combine them.

         
        • Iain Clarke

          Iain Clarke - 2020-04-03

          Thanks! That was a bit fiddly, and and maybe a notification or two would have made things easier, but it was workable! I think this can be closed as "not necessary".

           
  • Neil Hodgson

    Neil Hodgson - 2020-04-03

    There could be a notification when markers are moved or merged but that is a check that the aplication can perform on SC_MOD_DELETETEXT when linesAdded != 0. An API to iterate the handles on a line might help for applications that attach extra data to markers.

     
    • Iain Clarke

      Iain Clarke - 2020-04-03

      I used SCN_MODIFIED and filtered on lines added being non zero, as I needed to track markers moving as well as merging.

      But your suggestion of being able to do line->marker handles would have made my code simpler and less fragile.

      If that's thought to be a good thing, I don't mind putting in the work. I'm not scared of scintilla code!

      Iain.

      On 3 Apr 2020 23:50, Neil Hodgson nyamatongwe@users.sourceforge.net wrote:

      There could be a notification when markers are moved or merged but that is a check that the aplication can perform on SC_MOD_DELETETEXT when linesAdded != 0. An API to iterate the handles on a line might help for applications that attach extra data to markers.


      [feature-requests:#1344]https://sourceforge.net/p/scintilla/feature-requests/1344/ Add a pointer/LPARAM to each line, and a notification when a line is removed.

      Status: open
      Group: Initial
      Created: Wed Apr 01, 2020 07:54 PM UTC by Iain Clarke
      Last Updated: Fri Apr 03, 2020 08:13 AM UTC
      Owner: nobody

      The motivation is simple - I am writing a domain specific IDE, and am using Markers (SCI_MARKERADD etc) to mark breakpoints. This is great as far as it goes - if you add code/text in between, the markers move up and down to different line numbers along with the code they're associated with.

      But I want to add extra information to those breakpoints (ie, condition), and there's nowhere to store it. I could use some external place, with a mapping from line number to metadata, but that would be very fragile.

      Ideally, I'd want to attach it to the text line itself. This ability is all but in Scintilla. I could use Annotations, put information in them, and just make annotations invisible. But I would also like to use Annotations later for their given purpose later on.

      So, I would need something like:
      SCI_ADDLINEMETADATA (nLine, void *pLineMetadata).

      That I think is well within my ability to add, and if it was that simple, I'd have included a patch with this post. But we don't want leaks, so there would need to be some notification when a line (either always, or only if pLineMetadata != 0) is deleted, so the host application can delete the data, or some other clever thing.

      Or would something like:
      struct Metadata
      {
      uint nSize;
      byte pData
      }
      SCI_ADDLINEMETADATA (nLine, struct Metadata pLineMetadata);
      be better, where the metadata is copied into a blob stored with each line, and the original just passed in is the hosts problem?

      Then scintilla can tidy the line-metadata-blob when a line is removed, in the same way it tidies markers and annotations?

      I am happy to do the work, but I'd welcome some opinion on what would best fit with Scintilla's programming style/philosophy.


      Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/scintilla/feature-requests/1344/

      To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/

       

      Related

      Feature Requests: #1344

      • Neil Hodgson

        Neil Hodgson - 2020-04-04

        An API that returns the nth handle for a line, and out-of-bounds (-1) for n > number of handles would be fairly simple to implement and use if you want to work on it. Should include documentation in patch.

         
  • Iain Clarke

    Iain Clarke - 2020-04-04

    Attached is a patch adding two messages:
    SCI_MARKERHANDLEFROMLINE(line,which)
    and
    SCI_MARKERNUMBERFROMHANDLE(handle)

    The second message only exists because there weren't any more parameters to SCI_MARKERHANDLEFROMLINE. I saw very few structures in use for messages, so I kept this as two messages.

    Here's some code using it:

                    std::map<int, std::vector<int>> LinesToHandles;
                    for (int nLine = m_wndScintilla.SendMessage(SCI_MARKERNEXT, 0, 1 << DS_MARKNUM_BREAKPOINT); nLine >= 0; nLine = m_wndScintilla.SendMessage(SCI_MARKERNEXT, 1 + nLine, 1 << DS_MARKNUM_BREAKPOINT))
                    {
                        auto &v = LinesToHandles[nLine];
                        int nCount = 0;
                        for (int nHandle = m_wndScintilla.SendMessage(SCI_MARKERHANDLEFROMLINE, nLine, nCount); nHandle >= 0; nCount++, nHandle = m_wndScintilla.SendMessage(SCI_MARKERHANDLEFROMLINE, nLine, nCount))
                        {
                            // filter only handles we care about here
                            if (m_wndScintilla.SendMessage(SCI_MARKERNUMBERFROMHANDLE, nHandle) != DS_MARKNUM_BREAKPOINT)
                                continue;
                            v.push_back(nHandle);
                        }
                    }
    

    I also was not very sure which actual numbers to assign to the message defines, but that's trivial to change.

     
  • Neil Hodgson

    Neil Hodgson - 2020-04-04

    The API is defined in Scintilla.iface which is used to generate Scintilla.h by scripts/HFacer.py which also prints the current maximum message number (2731) so something like this should be in the patch:

    diff -r 5d8ef45011be include/Scintilla.iface
    @@ -156,6 +156,12 @@
     # Delete a marker.
     fun void MarkerDeleteHandle=2018(int markerHandle,)
    
    +# Retrieve marker handles of a line
    +fun int MarkerHandleFromLine=2732(line line, int which)
    +
    +# Retrieve marker number of a marker handle
    +fun int MarkerNumberFromHandle=2733(int markerHandle,)
    +
     # Is undo history being collected?
     get bool GetUndoCollection=2019(,)
    

    If the choice of SCI_MARKERNUMBERFROMHANDLE was just because of calling convention then it would likely be more efficient as

    fun int MarkerNumberFromLine=2733(line line, int which)
    

    since there are relatively few markers on most lines and MarkerNumberFromHandle iterates through a larger number of lines. APIs avoid struct arguments as structs are more difficult from scripting languages and there can also be 32/64-bit issues.

    For MarkerHandleSet::GetHandleByCount (and caller), the argument name markerNum is used in that file for the marker type or appearance which doesn't match this use - something conventional like index may be better.

     
    • Iain Clarke

      Iain Clarke - 2020-04-07

      I took your advice (it made sense!), and changed it appropriately (iface updated rather than scintilla.h.

      Code using the change has been updated to:

                      std::map<int, std::vector<int>> LinesToHandles;
                      for (int nLine = m_wndScintilla.SendMessage(SCI_MARKERNEXT, 0, 1 << DS_MARKNUM_BREAKPOINT); nLine >= 0; nLine = m_wndScintilla.SendMessage(SCI_MARKERNEXT, 1 + nLine, 1 << DS_MARKNUM_BREAKPOINT))
                      {
                          auto &v = LinesToHandles[nLine];
                          int nCount = 0;
      
                          while (1) {
                              int nHandle = m_wndScintilla.SendMessage(SCI_MARKERHANDLEFROMLINE, nLine, nCount);
                              int nMarker = m_wndScintilla.SendMessage(SCI_MARKERNUMBERFROMLINE, nLine, nCount);
                              if (nHandle < 0 || nMarker < 0)
                                  break;
      
                              if (nMarker == DS_MARKNUM_BREAKPOINT)
                                  v.push_back(nHandle);
      
                              nCount++;
                          };
                      }
      

      /Iain.

       
  • Neil Hodgson

    Neil Hodgson - 2020-04-08
    • labels: --> scintilla, markers
    • Group: Initial --> Committed
     
  • Neil Hodgson

    Neil Hodgson - 2020-04-08

    Committed after some minor changes, added tests, and running it through the astyle formatter as [665c31].

     

    Related

    Commit: [665c31]

  • Iain Clarke

    Iain Clarke - 2020-04-08

    Fame at last!

     
  • Neil Hodgson

    Neil Hodgson - 2020-04-27
    • status: open --> closed
     
  • Neil Hodgson

    Neil Hodgson - 2020-04-27

    Committed after some minor changes, added tests, and running it through the astyle formatter as [665c31].

     

    Related

    Commit: [665c31]


Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.