fix(eherbs.lic): v2.1.9 regex improvements

[mrhoribu]

Important

Improved regex in eherbs.lic for container lookups to match specific herb sack IDs, ensuring accurate content matching.

• Regex Improvements:
  • Updated regex in eherbs.lic to match existing herb sack ID in Utility.get_lines() calls for container lookups.
  • Affects regex patterns in EHerbs module for result, edible_result_contents, and liquid_result_contents.
  • Ensures accurate matching of container contents by using the specific herb sack ID.
• Version Update:
  • Incremented version from 2.1.8 to 2.1.9 in eherbs.lic to reflect changes.

^{This description was created by for d1c38a5. You can customize this summary. It will automatically update as commits are pushed.}

[mrhoribu]

@ellipsis remove previous comments and review again

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to d1c38a5 in 3 minutes and 5 seconds. Click for details.

• Reviewed 46 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. scripts/eherbs.lic:1975

• Draft comment:
  Consider wrapping the alternation in a non‐capturing group so that the entire pattern for container lookup is treated as one alternative.
• Reason this comment was not posted:
  Comment was on unchanged code.

2. scripts/eherbs.lic:2222

• Draft comment:
  Apply similar non‐capturing grouping for the alternation in the survival_contents regex to ensure correct matching.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The diff shows that the changes were specifically about replacing the generic ID capture (-?\d+) with a specific container ID interpolation (#{container.id}). The comment suggests making the first capture group non-capturing (?:In|On|Under|Behind), but this is unrelated to the actual changes made. The first capture group was already there before the changes and wasn't modified. This appears to be a general code quality suggestion rather than something related to the actual changes. According to the rules, comments should be about changes made by the diff, not about unchanged code. Additionally, making this change would shift the capture group numbers, which could break code that relies on Regexp.last_match(2), Regexp.last_match(3), etc. The comment might be technically correct that using non-capturing groups is a best practice, but it's not about the changes made in this PR. Also, changing capture group numbering could break existing code that uses numbered backreferences. This comment should be deleted because: 1) It's about unchanged code (the alternation pattern was already there), 2) Making this change would alter capture group indices and potentially break code that uses Regexp.last_match(n), and 3) It's a stylistic suggestion unrelated to the actual changes which were about using specific container IDs instead of generic ID patterns. Delete this comment. It's about code that wasn't changed in the diff, and implementing the suggestion could break existing code that relies on the current capture group numbering.

3. scripts/eherbs.lic:1975

• Draft comment:
  Consider using Regexp.escape on the interpolated container id to guard against potential regex injection if the id ever isn’t strictly numeric.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment raises a valid concern about regex injection. However, I need to consider: 1) Are container IDs always numeric in this context? Looking at GameObj in Lich, IDs are typically numeric strings from the game. 2) Is this speculative ("if the id ever isn't strictly numeric") or a real concern? The comment uses "if...ever" language which suggests speculation. 3) The original code explicitly matched numeric IDs with (-?\d+), and the new code interpolates the specific ID. If the ID is always numeric (which it should be from GameObj), then regex escaping wouldn't change behavior. 4) This feels like a "what if" scenario rather than identifying a concrete bug. The rules say not to make speculative comments like "If X, then Y is an issue". The comment is speculative ("if the id ever isn't strictly numeric"). In the Lich/GemStone context, GameObj IDs are always numeric strings provided by the game engine. The change from a generic numeric pattern to a specific ID interpolation is actually more restrictive, not less. If there were a real injection risk, we'd need to see evidence that non-numeric IDs can occur. While regex escaping is generally good practice, this comment is speculative about a scenario that shouldn't occur in this codebase. GameObj IDs are always numeric in the Lich environment. The comment uses conditional language ("if the id ever isn't") which indicates it's not identifying an actual issue but rather a hypothetical one. This violates the rule against speculative comments. This comment should be deleted. It's speculative about a scenario that doesn't apply to this codebase (non-numeric GameObj IDs), and uses "if...ever" language that indicates it's not identifying a concrete issue.

Workflow ID: wflow_4RAcbLHTBurRsMOO

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}