removed Warnings of non-standard Characters

[stschiff]

This is a trivial PR that simply removes the warnings for non-standard characters in PoseidonIDs and Group_Names.

@nevrome if you want you could try to implement a smarter forging language here that allows optional quoting, so that even names with non-standard characters can be used. If not, then this PR simply removes the warnings.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.10%. Comparing base (5aef90d) to head (d2fa8c2).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #390      +/-   ##
==========================================
+ Coverage   52.03%   52.10%   +0.06%     
==========================================
  Files          37       37              
  Lines        5994     5990       -4     
  Branches      644      642       -2     
==========================================
+ Hits         3119     3121       +2     
+ Misses       2231     2227       -4     
+ Partials      644      642       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[nevrome]

Here's a suggestion for the quoting. It's not the most clever implementation, but I think it's pretty predicable: Everything stays exactly as it is, but any entity name (!) can be quoted now. This makes it possible to have names with the forbidden characters ':', ',', '<', '>', '*'.

Examples of what should work now:

• Package: *"pa*c"*
• Group: "gro*up"
• Individual: <"indi>vidual">
• Specific individual: <"pac<kage":"gro<up":"indiv<idual">
• Package with version *"pack*age"-1.2.3*

I guess this would also allow names with spaces.

I realized that we're not well prepared for non-ASCII names, as we're parsing every name into a String. Maybe the warning you commented out now should still be thrown for everything that does not pass C.isAscii. For this case trident's behaviour is not tested and I would expect some of the unpleasant issues we did have back then in .janno files before we switched to Text. So a warning may genuinely be helpful.

[stschiff]

That's great! Thanks. To your last point though: String fully supports Unicode, so I think we're good for non-ASCII, aren't we?

[nevrome]

Didn't we have a long discussion about this for the .janno file, back then, where we realized that String is specifically not sufficient? This blog post explains the issue: https://hasufell.github.io/posts/2024-05-07-ultimate-string-guide.html

[stschiff]

OK, thanks for linking the Post again. It's indeed more complex than I thought to encode Unicode with Char and String, but certainly in principle possible without a problem. It may not be an ideal datatype for heavy Unicode applications, but I haven't read anything in this blog post that says that Strings can't be used for Unicode. Quite the contrary. Perhaps good to briefly chat about this.

Having said this, I have no problem with switching to Text at all, but I don't think this is of critical importance right now.

[stschiff]

OK, I left some inline comments. I don't understand P.skipMany (P.char '-') in L200 of EntityTypes.

[stschiff]

I'm very confused about this line. What exactly are we skipping here? Minuses? But they work fine in your tests...

[nevrome]

This is about the minus that separates the package name and the package version (for entities like **2029_Schiffels_Sulawesi-1.0.0*).

Look at the code before the confusing line: namePart <- parseQuoted <|> parseNamePart ""
parseNamePart is the old solution. It automatically consumes the - separating namePart and versionPart (if the versionPart exists) in the code following case nextChar of.
parseQuoted does not consume anything outside of the quotes, so the - stays there. So as the comment says for quoted case, where - is not yet handled we have to P.skipMany (P.char '-').