Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sipify.pl drops all but first word from comments #24609

Closed
qgib opened this issue Jun 13, 2017 · 15 comments
Closed

sipify.pl drops all but first word from comments #24609

qgib opened this issue Jun 13, 2017 · 15 comments
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! High Priority PyQGIS Related to the PyQGIS API

Comments

@qgib
Copy link
Contributor

qgib commented Jun 13, 2017

Author Name: Sandro Santilli (@strk)
Original Redmine Issue: 16710
Affected QGIS version: master
Redmine category:python_bindings_/_sipify
Assignee: Denis Rouzaud


This diff was introduced by git commit (which in turn calls sipify_all.sh:



--- a/python/core/geometry/qgsabstractgeometry.sip
+++ b/python/core/geometry/qgsabstractgeometry.sip
@@ -59,7 +59,7 @@ class QgsAbstractGeometry
 
     enum SegmentationToleranceType
     {
-      // Maximum external angle between segments, in radians
+      // Maximum
       MaximumAngle,
       MaximumDifference
     };


Basically I cannot write more than a single word in there, it gets truncated to one word on first git commit

@qgib
Copy link
Contributor Author

qgib commented Jun 13, 2017

Author Name: Sandro Santilli (@strk)


  • subject was changed from sipify_all.sh drops all but first word from comments to sipify.pl drops all but first word from comments

@qgib
Copy link
Contributor Author

qgib commented Jun 13, 2017

Author Name: Sandro Santilli (@strk)


It is sipify.pl doing the mungling, could be this block:

    # save comments and do not print them, except in SIP_RUN                    
    if ( $SIP_RUN == 0 ){                                                       
        if ( $LINE =~ m/^\s*\/\// ){                                            
            if ($LINE =~ m/^\s*\/\/\!\s*(.*?)\n?$/){                            
                $COMMENT = processDoxygenLine( $1 );                            
                $COMMENT =~ s/\n+$//;                                           
            }                                                                   
            elsif ($INPUT_LINES[$LINE_IDX-1] !~ m/\*\/.*/) {                    
                $COMMENT = '';                                                  
            }                                                                   
            next;                                                               
        }                                                                       
    }                                


  • assigned_to_id was changed from Jürgen Fischer to Denis Rouzaud

@qgib
Copy link
Contributor Author

qgib commented Jun 13, 2017

Author Name: Sandro Santilli (@strk)


The problem affected #4728

@qgib
Copy link
Contributor Author

qgib commented Jun 13, 2017

Author Name: Nyall Dawson (@nyalldawson)


I suspect it's because your doxygen tags are malformed. They should be:

enum SegmentationToleranceType
{
MaximumAngle, //!< Maximum external angle between segments, in radians
MaximumDifference
};

Note the important inclusion of the <, which is used whenever the doxygen comment comes AFTER the member (as is the case with enum values)

@qgib
Copy link
Contributor Author

qgib commented Jun 13, 2017

Author Name: Nyall Dawson (@nyalldawson)


  • status_id was changed from Open to Feedback

@qgib
Copy link
Contributor Author

qgib commented Jun 15, 2017

Author Name: Sandro Santilli (@strk)


Nyall moving the comment from above the enum value to its right side with '//!<' prefix makes it completely disappear from the .sip file.


  • status_id was changed from Feedback to Open

@qgib
Copy link
Contributor Author

qgib commented Jun 15, 2017

Author Name: Nyall Dawson (@nyalldawson)


That's by design - sip doesn't support documentation for enumeration values.

@qgib
Copy link
Contributor Author

qgib commented Jun 15, 2017

Author Name: Denis Rouzaud (@3nids)


indeed sip does not support docstrings for bindings and the policy is just to remove them from source.


  • status_id was changed from Open to Closed
  • category_id was changed from Build/Install to Python bindings / sipify

@qgib
Copy link
Contributor Author

qgib commented Jun 17, 2017

Author Name: Sandro Santilli (@strk)


If the policy is to remove them from output it is not working, as the very first word after the comment is being retained.


  • status_id was changed from Closed to Reopened

@qgib
Copy link
Contributor Author

qgib commented Jun 19, 2017

Author Name: Sandro Santilli (@strk)


So Nyall and Denis: do you think #4728 should be merged with the sip-generated broken comment in here: https://github.com/qgis/QGIS/pull/4728/files#diff-c57e05913a8dc3395f4b0f7d7f926293L61 ?

Those comments in .sip file basically become:

    enum SegmentationToleranceType
    {
      // Maximum
      // to
      MaximumAngle,
      // Maximum
      // curve
      MaximumDifference
    };

@qgib
Copy link
Contributor Author

qgib commented Sep 28, 2017

Author Name: Denis Rouzaud (@3nids)


  • status_id was changed from Reopened to Closed

@qgib
Copy link
Contributor Author

qgib commented Sep 28, 2017

Author Name: Sandro Santilli (@strk)


Why closed ? Was it fixed ? If so by which commit ?


  • status_id was changed from Closed to Reopened

@qgib
Copy link
Contributor Author

qgib commented Sep 28, 2017

Author Name: Denis Rouzaud (@3nids)


No issue here to me. The enum docs are not exported to sip files as they are not used.
As Nyall suggested you can use //!< which makes it a little nicer.

@qgib
Copy link
Contributor Author

qgib commented Sep 28, 2017

Author Name: Giovanni Manghi (@gioman)


Denis Rouzaud wrote:

No issue here to me. The enum docs are not exported to sip files as they are not used.
As Nyall suggested you can use //!< which makes it a little nicer.

so "won't fix"?

@qgib
Copy link
Contributor Author

qgib commented Sep 28, 2017

Author Name: Denis Rouzaud (@3nids)


  • resolution was changed from to wontfix
  • status_id was changed from Reopened to Closed

@qgib qgib closed this as completed Sep 28, 2017
@qgib qgib added Bug Either a bug report, or a bug fix. Let's hope for the latter! High Priority PyQGIS Related to the PyQGIS API labels May 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! High Priority PyQGIS Related to the PyQGIS API
Projects
None yet
Development

No branches or pull requests

1 participant