Bug 840228

Summary: YCP to Ruby translator destroyed some comments in the code
Product: [openSUSE] openSUSE Tumbleweed Reporter: Johannes Meixner <jsmeix>
Component: YaST2Assignee: E-mail List <yast2-maintainers>
Status: RESOLVED WONTFIX QA Contact: Jiri Srain <jsrain>
Severity: Major    
Priority: P5 - None    
Version: 13.1 Milestone 4   
Target Milestone: ---   
Hardware: All   
OS: SUSE Other   
Whiteboard:
Found By: Development Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---

Description Johannes Meixner 2013-09-13 09:44:14 UTC
I found some comments in the yast2-printer code
that got destroyed by the YCP to Ruby translator.

What I found up to now is that sometimes the first line
of a comment got lost or was misplaced elsewhere.

Examples:

In yast2-printer-2.23.1 YCP there was
in src/Printer.ycp (long lines wrapped here):
-------------------------------------------------------------------------------
  // what might be the reason why the server is not accessible via port 631.
  string netcat_test_fail_message = sformat( // Popup message
                                             // where %1 will be replaced by the server name.
                                             _("The server '%1' is not accessible via port 631 (IPP/CUPS)."),
                                             server_name
                                           );
  string ping_test_good_message = sformat( // Popup message
                                           // where %1 will be replaced by the server name.
                                           _("The server '%1' responds to a 'ping' in the network."),
                                           server_name
                                         );
-------------------------------------------------------------------------------
In yast2-printer-3.0.0 Ruby it has become
in src/modules/Printer.rb
-------------------------------------------------------------------------------
      # what might be the reason why the server is not accessible via port 631.
      netcat_test_fail_message = Builtins.sformat(
        # where %1 will be replaced by the server name.
        _("The server '%1' is not accessible via port 631 (IPP/CUPS)."),
        server_name
      ) # Popup message
      ping_test_good_message = Builtins.sformat(
        # where %1 will be replaced by the server name.
        _("The server '%1' responds to a 'ping' in the network."),
        server_name
      ) # Popup message

-------------------------------------------------------------------------------

In yast2-printer-2.23.1 YCP there was
in src/driveradd.ycp (long lines wrapped here):
-------------------------------------------------------------------------------
      if( hplip_install != hplip_installed )
      { if( hplip_install )
        { // When hpijs-standalone is installed, it must be removed first of all
          // because hpijs-standalone conflicts with hplip-hpijs which is required by hplip.
-------------------------------------------------------------------------------
In yast2-printer-3.0.0 Ruby it has become
in src/include/printer/driveradd.rb
-------------------------------------------------------------------------------
          if hplip_install != hplip_installed
            if hplip_install
              # because hpijs-standalone conflicts with hplip-hpijs which is required by hplip.
-------------------------------------------------------------------------------

In yast2-printer-2.23.1 YCP there was
in src/driveradd.ycp (long lines wrapped here):
-------------------------------------------------------------------------------
          { if( Printerlib::TestAndInstallPackage( "hplip", "install" ) )
            { package_changed = true;
            }
            else
            { // We are here either when the user has rejected to install hplip
              // or if the installation of hplip has actually failed.
-------------------------------------------------------------------------------
In yast2-printer-3.0.0 Ruby it has become
in src/include/printer/driveradd.rb
-------------------------------------------------------------------------------
                if Printerlib.TestAndInstallPackage("hplip", "install")
                  package_changed = true
                else
                  # or if the installation of hplip has actually failed.
-------------------------------------------------------------------------------

In yast2-printer-2.23.1 YCP there was
in src/driveradd.ycp (long lines wrapped here):
-------------------------------------------------------------------------------
                    if( Printerlib::TestAndInstallPackage( "hpijs-standalone", "installed" ) )
                    { // It can lead to an almost endless sequence of further problems
                      // when hpijs-standalone is removed by the package management system
-------------------------------------------------------------------------------
In yast2-printer-3.0.0 Ruby it has become
in src/include/printer/driveradd.rb
-------------------------------------------------------------------------------
                        if Printerlib.TestAndInstallPackage(
                            "hpijs-standalone",
                            "installed"
                          )
                          # when hpijs-standalone is removed by the package management system
-------------------------------------------------------------------------------

There are many more examples as above in yast2-printer.
Comment 1 David Majda 2013-09-13 12:12:20 UTC
Taking, will investigate.
Comment 2 Johannes Meixner 2013-09-13 14:49:39 UTC
By a hacky ad-hoc script I found a lot of comments got lost:

------------------------------------------------------------------------------
$ grep -o '// .*' yast2-printer-2.23.1/src/Printer.ycp \
  | cut -s -d ' ' -f2- \
  | while read LINE ; \
    do SEARCH=$( echo $LINE | tr -d '\n' | tr -c '[:alnum:]' '.' ) ; \
    grep -q "$SEARCH" yast2-printer-3.0.0/src/modules/Printer.rb 
      || echo MISSING: $LINE; \
    done \
  | head

MISSING: Show feedback because creating the printer driver database takes several seconds:
MISSING: Show nice self-updating feedback:
MISSING: Show a simple static busy message:
MISSING: No title for a simple busy message:
MISSING: Setting the DownloadProgress to 100% by setting its property
MISSING: Show feedback because detecting printers takes a few seconds:
MISSING: Show nice self-updating feedback:
MISSING: Show a simple static busy message:
MISSING: No title for a simple busy message:
MISSING: Setting the DownloadProgress to 100% by setting its property

$ grep -o '// .*' yast2-printer-2.23.1/src/Printer.ycp \
  | cut -s -d ' ' -f2- \
  | while read LINE ; \
    do SEARCH=$( echo $LINE | tr -d '\n' | tr -c '[:alnum:]' '.' ) ; \
    grep -q "$SEARCH" yast2-printer-3.0.0/src/modules/Printer.rb 
      || echo MISSING: $LINE; \
    done \
  | wc -l

105
------------------------------------------------------------------------------

It sems 105 comments got lost during YCP to Ruby translation
in one single file src/Printer.ycp -> src/modules/Printer.rb
Comment 3 Johannes Meixner 2013-09-13 14:58:19 UTC
It seems all missing comments are in YCP lines of the form

... { // comment ...

or

... ( // comment ...

because

$ grep -o '// .*' yast2-printer-2.23.1/src/Printer.ycp \
  | cut -s -d ' ' -f2- \
  | while read LINE ; \
    do SEARCH=$( echo $LINE | tr -d '\n' | tr -c '[:alnum:]' '.' ) ; \
    grep -q "$SEARCH" yast2-printer-3.0.0/src/modules/Printer.rb \
      || grep "$SEARCH" yast2-printer-2.23.1/src/Printer.ycp ; \
    done \
  | egrep -v '\{ // |\( //'

results nothing.
Comment 4 David Majda 2014-08-18 06:42:03 UTC
I won't be fixing this bug any time soon => reassigning to yast2-maintainers@suse.de.
Comment 5 Lukas Ocilka 2014-08-25 13:16:31 UTC
Johannes, I'm afraid that we will NOT be able to fix YCP Killer to
fix printer code. The translation from YCP to Ruby has been done already
and there is no way to do it again as changes were made to the code
already.

We could investigate it and fix it for translations in the future, though.
Do you expect us to fix printer?
Comment 6 Johannes Meixner 2014-08-25 14:18:15 UTC
I think meanwhile it has become in practice too late to fix it.

Furthermore - if I ever may get time - I am planning to re-write it
from scratch (now in plain Ruby) because the current one becomes
more and more antiquated, see https://features.opensuse.org/316789
"Fundamental re-design how to set up the whole printing stuff"

Therefore I close this bug as "wontfix" (i.e. I do no longer need a fix).

But this issue could be serious for others who look at the code
because when the first line of a comment is lost, the rest of
the comment could become incomprehensible/meaningless/wrong:
For example from YCP
------------------------------------------------------------------
 { // Use that_func because it will terribly crash when you
   // use this_func in this particular case to make it working:
   that_func() ;
 }
------------------------------------------------------------------
to YaST-Ruby
------------------------------------------------------------------
 # use this_func in this particular case to make it working:
 Builtins.that_func()
------------------------------------------------------------------
;-)