Opened 3 years ago

Closed 3 years ago

#17680 closed enhancement (fixed)

Some proposals for wxSTC documentation

Reported by: NewPagodi Owned by: Artur Wieczorek <artwik@…>
Priority: normal Milestone: 3.1.1
Component: wxStyledText Version: dev-latest
Keywords: documentation Cc:
Blocked By: Blocking:
Patch: yes

Description

I've attempted to complete some of the todo items in the documentation for wxStyledTextCtrl and wxStyledTextEvent. While doing research for this, I made a general observation:

I'm almost certain that the events wxEVT_STC_KEY and wxEVT_STC_URIDROPPED are not being generated. Should they be removed from the class maybe?


For wxStyledTextEvent, I've added the event list, slightly reordered it to match the scintilla documentation, added descriptions for most of the events matching the style of the existing descriptions, and alphabetized and added descriptions for the accessors. The one event I wasn't completely sure about was wxEVT_STC_DRAG_OVER - I based the list of valid functions on looking at the code and experiments with the event.

I've temporarily put up a sample at https://newpagodi.github.io/tempstcdocs/classwx_styled_text_event.html . The resulting event list is very cluttered, but I'm not sure what can be done about it. If this is acceptable, I can submit a patch.


For wxStyledTextCtrl, since there are so many methods, I thought it would be helpful to break them into groups similar to the scintilla documentation. I based the page layout on the wxString documentation, and have a sample here https://newpagodi.github.io/tempstcdocs/classwx_styled_text_ctrl.html .

I made this sample by manually editing the interface file (and the groupings are not correct or complete), but I think I can edit gen_iface.py to make it output something similar. I'll need to add a map of all the methods to categories. And since that has to be done, I can add 2 other things to the map for each method,

1) an optional set of extended documentation. This can be used, for example, to add @since fields to recently added methods.
2) a optional set of string replacements. For example the documentation for SetEdgeMode that is automatically generated is:

The edge may be displayed by a line (EDGE_LINE) or by highlighting
text that goes beyond it (EDGE_BACKGROUND) or not displayed at all
(EDGE_NONE).

This docline includes the macros EDGE_LINE, EDGE_BACKGROUND, and EDGE_NONE that are defined in scintilla.h. For use with wxSTC, the docline should be

The edge may be displayed by a line (wxSTC_EDGE_LINE) or by
highlighting text that goes beyond it (wxSTC_EDGE_BACKGROUND)
or not displayed at all (wxSTC_EDGE_NONE).

This will be a lot of work, but if this is an acceptable change, I'm willing to give it a try.

Attachments (2)

stcdocs.patch download (123.1 KB) - added by NewPagodi 3 years ago.
stcdocs-part2.patch download (188.8 KB) - added by NewPagodi 3 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 3 years ago by NewPagodi

I had a second idea for wxStyledTextEvent. Have a section describing the event types before the event list. Here's a first draft showing this layout: https://newpagodi.github.io/tempstcdocs/classwx_styled_text_event2.html .

I think this declutters the event list and makes it look like those in most of the other event classes, but it does make the page longer.

comment:2 Changed 3 years ago by NewPagodi

  • Patch set

Here's the latest version:

wxStyledTextCtrl: https://newpagodi.github.io/tempstcdocs/classwx_styled_text_ctrl3.html

wxStyledTextEvent: https://newpagodi.github.io/tempstcdocs/classwx_styled_text_event3.html

For wxStyledTextCtrl, I've

  • added a table of contents
  • broke the methods into sections
  • added docs for 28 previously undocumented methods
  • altered 3 docs that were unhelpful or misleading
  • added 56 @since 3.1.0 notes and added 11 @since 3.1.1 notes
  • altered 18 docs which contained references to scintilla macros or other problematic text.

For wxStyledTextEvent, I've:

  • added a section describing the event generation and relevant functions
  • documented the methods.

I think this makes the documentation more organized, complete, correct, and hopefully more helpful.

I'm attaching a patch. I had to make substantial changes to gen_iface.py and stc.interface.h.in, and I also had to make very slight changes to stc.h.in and stc.cpp.in. I can provide a list of changes and the rationale for them if needed.

One thing I noticed is that when methods are categorized, if any of them are undocumented, doxygen will reiterate the docline for the first entry in the category. I'm not sure if that's bug or a feature. Because of this, one of the modifications to gen_iface.py prints out a warning if a method doesn't have documentation.

comment:3 Changed 3 years ago by vadz

  • Milestone set to 3.1.1
  • Status changed from new to confirmed

Thanks a lot for working on this! I'm (unfortunately not unusually) very busy right now and I don't know have that much experience with wxSTC, so if others could please review the patch and documentation generated after applying it and leave notes about their experience here, I'd be more than glad to apply this.

Thanks again!

comment:4 follow-up: Changed 3 years ago by awi

  • Status changed from confirmed to infoneeded_new

Some remarks:

  1. Attached patch has lots of trailing spaces (in more than 450 lines) what triggers git warnings and blocks of committing the changes to the repository.
  2. Modified gen_iface.py fails on executing (Python 3.5.3) with following error:
    Traceback (most recent call last):
      File "gen_iface.py", line 2285, in <module>
        main(sys.argv)
      File "gen_iface.py", line 2280, in main
        processIface(IFACE, H_TEMPLATE, CPP_TEMPLATE, IH_TEMPLATE, H_DEST, CPP_DEST,
     DOCSTR_DEST, IH_DEST, msgcodes)
      File "gen_iface.py", line 1935, in processIface
        defs, imps, docstrings, idefs = processMethods(methods)
      File "gen_iface.py", line 2019, in processMethods
        category, docs, docsLong = buildDocs(interfName, docs)
      File "gen_iface.py", line 2132, in buildDocs
        for x,y in postProcess.iteritems():
    AttributeError: 'dict' object has no attribute 'iteritems'
    
  3. I think it would be good to move changes made to stc.cpp.in and stc.h.in to the separate patch and provide some rationale for them (which APIs were added, removed, why, etc.). This would be helpful in preparing well commented commit to the repository.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 3 years ago by NewPagodi

  • Status changed from infoneeded_new to new

Replying to awi:

Some remarks:

  1. Attached patch has lots of trailing spaces (in more than 450 lines) what triggers git warnings and blocks of committing the changes to the repository.

Thanks for looking into this ticket. Sorry about the extra spaces. Ironically, that was a scintilla issue due to cutting and pasting, but I should have caught that. I'll fix that in a new patch.


  1. Modified gen_iface.py fails on executing (Python 3.5.3) with following error:

I didn't realize gen_iface.py worked with python3. I thought it only worked with python2. I think I know how to iterate over dictionaries in a way that works in both 2 and 3. I'll try to fix that too.


  1. I think it would be good to move changes made to stc.cpp.in and stc.h.in to the separate patch and provide some rationale for them (which APIs were added, removed, why, etc.). This would be helpful in preparing well commented commit to the repository.

Less than a month after I submitted this, the scintilla team released version 3.7.0 which finally documented all the methods in Scintilla.iface. Since then they've also had 2 more small updates

So maybe

  1. Let me try to redo #17705 to update wxSTC to scintilla 3.7.2. That will significantly reduce the number of methods that need separate documentation in this patch.
  2. Decide what to do about #17688. The problem there is that 2 events are never generated. I think the best solution is to simply remove the reference to those events from the documentation.
  3. Then return to this ticket. Maybe the best thing would be to split it into three separate patches: a patch for updating the wxStyledTextEvent documentation, a patch for stc.h.in and stc.cpp.in, and a third patch for gen_iface.py.
  4. redo #17671 to handle deprecated items .

Is this an acceptable stategy?

comment:6 in reply to: ↑ 5 Changed 3 years ago by awi

Replying to NewPagodi:
That's fine by me.

comment:7 follow-up: Changed 3 years ago by NewPagodi

I've updated the patch to address the issues above (actually I had to split it into 2 pieces since it completely reorders the interface header, the resulting diff is quite large and exceeds the upload limit).

A sample of the documentation produced can be seen here https://newpagodi.github.io/tempstcdocs/classwx_styled_text_ctrl5.html.

The patch now doesn't have extra whitespace and works with both python 2 and 3. With #17800 and #17803, hopefully this is now easier to evaluate. This now mostly changes gen_iface.py, but I also

  • had to make 1 change and 2 deletions in stc.interface.h.in,
  • changed README.txt to request that the new structure I've added be updated when the scintilla library is updated.

I made a few additional changes to gen_iface.py from the previous version

  • I've added a few more postprocessing items to remove more scintilla macros.
  • A number of the docstrings in the iface file include lines like 'Result is NUL-terminated.' Since wxSTC uses wxString, this isn't true, so I've used postprocessing to change those lines to empty strings.
  • The only remaining function in the iface file without a docstring is GetXOffset, so I added a docstring for it and altered the docstring for SetXOffset.
  • In the previous version, I just copied all of the documentation changes from methodOverrideMap to the new docMap. This time I looked over those items and I made adjustments for the following 5 items:
    • GetMultiPaste and GetPropertyInt were identical to the iface documentation. So I didn't copy those 2 over.
    • For GetProperty, GetPropertyExpanded, and ReplaceTargetRE, I used the postprocessing to create the the docstring that had been in methodOverrideMap from the strings in the iface file.
Last edited 3 years ago by NewPagodi (previous) (diff)

comment:8 Changed 3 years ago by NewPagodi

After I submitted this, I noticed some of the names for some macros/constants aren't what they should be. I've opened #17807 to address this. If that ticket is accepted, I'll resubmit this again.

In the meantime, can I ask about another idea I just had? tl/dnr version:
are there any objections to me adding 4 or 5 new structures to help in generating the docstrings and/or adding a new file to the stc source folder?

longer version:

One of the tasks included in README.txt for updating the Scintilla library is "check any documentation-only changes from Scintilla.iface and see if the existing docs for those items should be updated too." That's really important, but it's not easy to do either currently when the existing docs are spread out in methodOverrideMap or in my proposed change where they'll be spread out in docsMap.

So I was thinking instead of instead of lumping everything all the docstring changes into a single dictionary structure, maybe have 1 structure to categorize the items, another to provide overrides for documentation in the iface file, another for extended doc strings and so on.

Except for the categorization structure, most of the new structure should be pretty short. But it will still add several new pieces to clutter up gen_iface.py. So I think it might be better to add the new structures to a separate file named something like stc_docs.py.

comment:9 in reply to: ↑ 7 ; follow-up: Changed 3 years ago by awi

  • Status changed from new to infoneeded_new

Replying to NewPagodi:

I've updated the patch to address the issues above (actually I had to split it into 2 pieces since it completely reorders the interface header, the resulting diff is quite large and exceeds the upload limit).

It seems that stc.h file generated by the script contains trailing spaces after two slash marks:

    // Retrieve the text of the line containing the caret.
    // 
    // 

Maybe this piece of code can be blamed for this effect?

            for x in docs:
                defs.append('    // ' + x)

comment:10 in reply to: ↑ 9 Changed 3 years ago by NewPagodi

  • Status changed from infoneeded_new to new

Replying to awi:

It seems that stc.h file generated by the script contains trailing spaces after two slash marks:

    // Retrieve the text of the line containing the caret.
    // 
    // 

Maybe this piece of code can be blamed for this effect?

            for x in docs:
                defs.append('    // ' + x)

I've updated the patch to fix this problem. I've also added extended documentation strings to many of the methods that take or return one of the constants defined in stc.h to indicate which of the constants are relevant.

I've also moved all the doc stuff to a separate file I called "gen_docs.py". I hope this is acceptable because otherwise gen_iface.py will be really cluttered.

The new structures in the separate file are:
categoriesList - a list of the categories and an optional description.
docsMap - a dictionary that assigns each method to one of the categories.
docOverrides - a dictionary that can be used to replace the docstring from the iface file.
docSubstitutions - a dictionary that can be used to replace a few words from the docstring in the iface file.
extendedDocs - a dictionary that can be used to add extended documentation.
sinceAnnotations - a dictionary that can be used to add @since annotations.

Hopefully it's obvious how to use them if someone else wants to add to them to change documentation in the future.

Incidentally, if in the future someone updates the Scintilla library but doesn't update the docsMap, any new methods will be listed in the "Other Settings" section. That's not the ideal place, but they will be listed some place.

comment:11 Changed 3 years ago by evstevemd

wxSTC documentation have not been good enough, glad someone is taking a shot at it. There is yellowbrain documentationand is really comprehensive about many things. Are you aware of it?

comment:12 follow-up: Changed 3 years ago by awi

I really like the idea of moving of all doc-related stuff to the separate module. Definitely, the code is more clear now. I hope nobody will challenge this concept.
Also, the contents is much more complete than before. And it has a structure. It really looks better!

Some minor remarks:

  1. Maybe it would be good to note somewhere in the readme file about compatibility with Python? For instance, it seems that sys.dont_write_bytecode is available since 2.6 and maybe this can matter on some platforms. I have no idea on what platforms this script can be executed, what Python versions can be expected, so maybe such remark could be helpful.
  1. Deprecated functions would go now to the "Deprecated" category in the documentation, but in the stc.h header file they are not marked as deprecated (AFAICS). I think it would be nice to have synchronized deprecated functions in the documentation and wxSTC header. But maybe this is a task for #17671?

comment:13 in reply to: ↑ 12 Changed 3 years ago by NewPagodi

Replying to awi:

Some minor remarks:

  1. Maybe it would be good to note somewhere in the readme file about compatibility with Python? For instance, it seems that sys.dont_write_bytecode is available since 2.6 and maybe this can matter on some platforms. I have no idea on what platforms this script can be executed, what Python versions can be expected, so maybe such remark could be helpful.

Correct me if I'm wrong, but I think the script will still run if the version is earlier than 2.6. But since there's no existing item sys.dont_write_bytecode, trying to set it True will have no effect and a .pyc file will be generated. So I added this comment to the readme:

7. Run gen_iface.py.  It's best to use python 2.6 or later.  If
using an earlier version, please delete any .pyc files generated.

If I'm wrong or a different comment would be better, I can change it.


  1. Deprecated functions would go now to the "Deprecated" category in the documentation, but in the stc.h header file they are not marked as deprecated (AFAICS). I think it would be nice to have synchronized deprecated functions in the documentation and wxSTC header. But maybe this is a task for #17671?

Yes, I thought it would be best to redo the patch in #17671 to handle this since that will require some additional changes to gen_iface.py

Changed 3 years ago by NewPagodi

Changed 3 years ago by NewPagodi

comment:14 Changed 3 years ago by Artur Wieczorek <artwik@…>

  • Owner set to Artur Wieczorek <artwik@…>
  • Resolution set to fixed
  • Status changed from new to closed

In 2278bb0dfa4aaa252930b7fd5e609f379f1113a6/git-wxWidgets:

Update wxSTC documentation stuff

  1. All stuff to generate documentation is moved from 'gen_iface.py' to a separate module 'gen_docs.py'. It contains several new structures to support maintenance of the documentation: categoriesList - A list of the categories and an optional description. docsMap - A dictionary that assigns each method to one of the categories. docOverrides - A dictionary that can be used to replace the docstring from the iface file. docSubstitutions - A dictionary that can be used to replace a few words from the docstring in the iface file. extendedDocs - A dictionary that can be used to add extended documentation. sinceAnnotations - A dictionary that can be used to add '@since' annotations.
  1. Documentation is reformatted and updated:
    • Added a table of contents.
    • Broke the methods into sections.
    • Added docs for previously undocumented methods.
    • Altered docs that were unhelpful or misleading.
    • Added '@since 3.1.0' notes and '@since 3.1.1' notes.
    • Altered docs which contained references to Scintilla macros or other problematic text.

Closes #17680.

Note: See TracTickets for help on using tickets.