Converting text to a label can create a label with illegal characters

[Chris Elliott]

While confirming a potential bug on the forum I discovered that I can create a label with spaces (I don't recall off the top of my head any of the other illegal label characters, so I didn't try them).

When normally creating a label the text input box doesn't insert anything when the space bar is pressed. Also, if when creating a label one is able to get a space into the text input box by pasting a string containing spaces, an error is thrown when the OK button is clicked. Both of these behaviors are expected.

One can create a normal text that includes spaces. (This is also expected.) BUT, then one can convert this text into a label, and now one has a label with spaces. ERC doesn't flag this. If one edits this label-with-spaces the OK button will throw an error, but the cancel button doesn't. An error should be thrown when first attempting to convert the text to a label if the text contains illegal characters for labels.

See the attached screenshot of a label (indicated by black character color) with spaces.

My version info:
Application: kicad
Version: (5.1.2)-1, release build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.61.1 OpenSSL/1.1.1 (WinSSL) zlib/1.2.11 brotli/1.0.6 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.5) nghttp2/1.34.0
Platform: Windows 8 (build 9200), 64-bit edition, 64 bit, Little endian, wxMSW
Build Info:
    wxWidgets: 3.0.4 (wchar_t,wx containers,compatible with 2.8)
    Boost: 1.68.0
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.61.1
    Compiler: GCC 8.2.0 with C++ ABI 1013

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_PYTHON3=OFF
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_WXPYTHON_PHOENIX=OFF
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

[Michael Kavanagh (michaelkavanagh)]

This appears fixed in the master branch because the context menu option to change type is missing...

[Wayne Stambaugh (stambaughw)]

I took a look at this issue and it looks like we attempted to address part of the problem by no adding the convert text submenu when the text object contains a line feed character. Obviously there are other characters are not valid label characters other than the line feed character. The question is, how to address this? I could check for all invalid label characters and prevent the convert text submenu or I could always show the convert subment and replace all invalid characters with and underscore or some other valid character. I prefer the latter as it is confusing to show the conversion submenu under certain conditions but not others. Anyone else have any thoughts on this before I fix the issue?

The missing context menu issue is a separate bug report. I suspect this just got dropped from the conversion to the new tool framework but it definitely needs to be fixed. I will file a new bug report.

[Chris Elliott (sembazuru)]

Instead of automatically changing things without warning to the users, maybe run through the same validation process when the OK button of the label properties dialog is pressed (see attachment). It would be nice if the warning message included why the validation failed. Maybe a list of invalid characters? Depending on how flexible the validation warning dialog is, maybe an additional button to replace invalid characters with underscore? This gives the user a choice (yes replace, or no let the user handle it in their own fashion).

[Jon Evans (craftyjon)]

@Wayne, I'm making a custom validator for net names for V6, it may be possible to backport in case it's helpful here.

[Wayne Stambaugh (stambaughw)]

@Chris, most users aren't big nagware fans so that's why I didn't add a warning dialog. Most users know what characters are valid in labels so it should not be a surprise when converting a text object to a label. That being said, if it's really necessary, we can add a warning dialog with an option to permanently disable showing the dialog.

@Jon, If you do create a custom validator for net names, please make sure it can be cherry picked to 5.1. Do we need a separate label validator or is the net name validator sufficient?

[KiCad Janitor (kicad-janitor)]

Fixed in revision 691f05273e3e90852fe9fca9808b916e9cc9a1ff
https://git.launchpad.net/kicad/patch/?id=691f05273e3e90852fe9fca9808b916e9cc9a1ff

[Chris Elliott (sembazuru)]

@Wayne, Yeah, there is a very fine line between an informational warning message and nagware.

But, IMHO, my suggestion is just on the side of informational warning of that line. I don't think it would be a super common warning to get, most common when converting a text to a label. An obtuse message just saying "No" or something unexpected happening like unrequested character substitution I think would be more disruptive than an informative warning message. Though I could be biased because it is my suggestion. ;-)