Service spec possibly wrongly generated for string constant with comment character

[StefanFabian]

I am currently working on a library that allows the sending and receiving of messages in C++ without knowing their types at compile time.
While reviewing the code that generates the message definition and md5 sum (which is very well written and easy to read btw) to replicate it in C++, I came across the load_srv_from_string function

def load_srv_from_string(msg_context, text, full_name):
    """
    Load :class:`SrvSpec` from the .srv file.
    
    :param msg_context: :class:`MsgContext` instance to load request/response messages into.
    :param text: .msg text , ``str``
    :param package_name: context to use for msg type name, i.e. the package name,
      or '' to use local naming convention. ``str``
    :returns: :class:`SrvSpec` instance
    :raises :exc:`InvalidMsgSpec` If syntax errors or other problems are detected in file
    """
    text_in  = StringIO()
    text_out = StringIO()
    accum = text_in
    for l in text.split('\n'):
        l = l.split(COMMENTCHAR)[0].strip() #strip comments        
        if l.startswith(IODELIM): #lenient, by request
            accum = text_out
        else:
            accum.write(l+'\n')

    # create separate MsgSpec objects for each half of file
    msg_in = load_msg_from_string(msg_context, text_in.getvalue(), '%sRequest'%(full_name))
    msg_out = load_msg_from_string(msg_context, text_out.getvalue(), '%sResponse'%(full_name))
    return SrvSpec(msg_in, msg_out, text, full_name)

I haven't tested it which is why I said possibly in the title but wouldn't the comment stripping result in the incorrect removal of parts of valid constant definitions such as

string EXAMPLE="#comments" are ignored, and leading and trailing whitespace removed

taken from the msg wiki.

[dirk-thomas]

That looks indeed to be the case. Default values were introduced much later so this logic predates that and isn't aware of default values.

[flixr]

@StefanFabian it would be great if you could make a PR with a test and the corresponding fix!

[StefanFabian]

Sure, it will take maybe a week or two, though, since I have to finish writing tests for a rospack PR first.

[StefanFabian]

That's why you shouldn't give time estimates ^^ but #92 should fix this.