Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add code to escape member names when doing resolve #1655

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

worksofliam
Copy link
Contributor

Changes

Fixes issue where member resolves with dollar signs in the name (which sadly is very common) was not working because $ is a special character in the shell.

Unit tests for memberResolve are continung to pass.

Checklist

  • have tested my change
  • updated relevant documentation
  • Remove any/all console.logs I added
  • eslint is not complaining
  • have added myself to the contributors' list in CONTRIBUTING.md
  • for feature PRs: PR only includes one feature enhancement.

src/api/IBMiContent.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@sebjulliand sebjulliand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the member name be escaped as well?

@worksofliam worksofliam added this to the 2.5.0 release milestone Nov 19, 2023
@worksofliam
Copy link
Contributor Author

@sebjulliand Take another look when you can - thanks!

Copy link
Collaborator

@sebjulliand sebjulliand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect; code is clear and it works fine! Let's merge.

@sebjulliand sebjulliand merged commit db5bb99 into master Nov 21, 2023
1 check passed
@sebjulliand sebjulliand deleted the fix/escape_dollar_signs_in_lookup branch November 21, 2023 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants