Repository: KLayout/klayout (language: python)

Issue:
how to specify an empty string for strm2oas --subst-char ?
In klayout, if the substitution string is "empty" illegal characters in TEXT will not be substituted.

![image](https://github.com/user-attachments/assets/98891d75-31d9-48a5-87a6-de0b6948c22b)

How can I specify an empty string for strm2oas on the command line ?

strm2oas --subst-char    test_illegal_char/test.oas a.oas 
ERROR: Additional arguments required (use -h for help)

strm2oas --subst-char "" test_illegal_char/test.oas a.oas
ERROR: Additional arguments required (use -h for help)

strm2oas --subst-char '' test_illegal_char/test.oas a.oas
ERROR: Additional arguments required (use -h for help)

strm2oas --subst-char \\"\\" test_illegal_char/test.oas a.oas
This is the only one not giving an ERROR, but uses * as substitution character.
TEXTSTRING "illegal*"

I had to gzip the .oas file to be able to attach it, please gunzip, it's a strict oas.
[test.oas.gz](https://github.com/user-attachments/files/17331250/test.oas.gz)




Discussion / hints:
I would prefer the "default" to be "don't replace anything", one could use --subst-char "*" to get the current behavior.
A warning about "illegal" characters would be nice either way.
Hi Stefan,

Looks like the parser has trouble with passing empty strings.

The "don't substitute" default behavior proposal is fine with me as long as it applies to "strm2oas". For the GUI application I think, the default should be some safe value. But it could be made optional, so you can turn off character substitution.

Matthias




I always expect any tool to write exactly what it read by default.
That even applies to “illegal” oasis, garbage in garbage out.
Especially since “other tools” aka the $$$ tools allow one do this too.

Currently I can’t switch off strm2oas subst-char, in klayout I can do that.

I believe the best solution is to change strm2oas default to “don’t substitute”.
One can use —subst-char “*” if one wants to substitute. A warning would be nice.
gds doesn't have the concept of "illegal characters".
I've never seen them used in cell names, only in TEXT, they won't really do any harm there.

They were there either because a certain layout tools allows \<tab\> or \<cr\> to be entered in their text entry form and people created multi-line text entries.
Or because a certain fab used binary numbers in the layer:datatype 63:63 IPTAG TEXT as some sort of signature in their stdcell gds.

In both cases, it would be much preferred if a round trip gds->oasis->gds "keeps the TEXT as it was".

Fix patch:
diff --git a/src/buddies/src/bd/bdWriterOptions.cc b/src/buddies/src/bd/bdWriterOptions.cc
index a18e75e871..440d7ba653 100644
--- a/src/buddies/src/bd/bdWriterOptions.cc
+++ b/src/buddies/src/bd/bdWriterOptions.cc
@@ -57,7 +57,8 @@ GenericWriterOptions::GenericWriterOptions ()
   m_oasis_recompress = save_options.get_option_by_name ("oasis_recompress").to_bool ();
   m_oasis_permissive = save_options.get_option_by_name ("oasis_permissive").to_bool ();
   m_oasis_write_std_properties = save_options.get_option_by_name ("oasis_write_std_properties").to_int ();
-  m_oasis_subst_char = save_options.get_option_by_name ("oasis_substitution_char").to_string ();
+  //  No substitution by default (issue #1885), so skip this:
+  //  m_oasis_subst_char = save_options.get_option_by_name ("oasis_substitution_char").to_string ();
 
   m_cif_dummy_calls = save_options.get_option_by_name ("cif_dummy_calls").to_bool ();
   m_cif_blank_separator = save_options.get_option_by_name ("cif_blank_separator").to_bool ();
@@ -310,6 +311,8 @@ void GenericWriterOptions::set_oasis_substitution_char (const std::string &text)
 {
   if (! text.empty ()) {
     m_oasis_subst_char = text[0];
+  } else {
+    m_oasis_subst_char = std::string ();
   }
 }
 


Test patch:
diff --git a/src/buddies/unit_tests/bdBasicTests.cc b/src/buddies/unit_tests/bdBasicTests.cc
index 5e67484092..86c71f7335 100644
--- a/src/buddies/unit_tests/bdBasicTests.cc
+++ b/src/buddies/unit_tests/bdBasicTests.cc
@@ -313,3 +313,41 @@ TEST(11)
   EXPECT_EQ (stream_opt.get_option_by_name ("cell_conflict_resolution").to_string (), "RenameCell");
 }
 
+//  Testing writer options
+TEST(12_issue1885)
+{
+  bd::GenericWriterOptions opt;
+  tl::CommandLineOptions cmd;
+
+  opt.add_options (cmd);
+
+  db::Layout layout;
+
+  db::SaveLayoutOptions stream_opt;
+  opt.configure (stream_opt, layout);
+
+  EXPECT_EQ (stream_opt.get_option_by_name ("oasis_substitution_char").to_string (), "");
+
+  const char *argv[] = {
+                   "x",
+                   "--subst-char=x",
+                 };
+
+  cmd.parse (sizeof (argv) / sizeof (argv[0]), const_cast<char **> (argv));
+
+  opt.configure (stream_opt, layout);
+
+  EXPECT_EQ (stream_opt.get_option_by_name ("oasis_substitution_char").to_string (), "x");
+
+  const char *argv2[] = {
+                   "x",
+                   "--subst-char=",
+                 };
+
+  cmd.parse (sizeof (argv2) / sizeof (argv2[0]), const_cast<char **> (argv2));
+
+  opt.configure (stream_opt, layout);
+
+  EXPECT_EQ (stream_opt.get_option_by_name ("oasis_substitution_char").to_string (), "");
+}
+