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

string.replace IntelliSense is conveying wrong meaning #41361

Open
Bharatwasi opened this issue Nov 2, 2020 · 9 comments · May be fixed by #41385
Open

string.replace IntelliSense is conveying wrong meaning #41361

Bharatwasi opened this issue Nov 2, 2020 · 9 comments · May be fixed by #41385

Comments

@Bharatwasi
Copy link

@Bharatwasi Bharatwasi commented Nov 2, 2020

TypeScript Version: 4.1.0-beta

Search Terms: string.replace

Code

const str ='apple apple';
console.log(str.replace('apple','oranges'));

Expected behavior: It should replace all occurrences of 'apple'

Actual behavior: It is replacing only the first occurrence of 'apple'.
Either correct the IntelliSense doc or fix the replace function. Please find the attached screenshot below
image

Playground Link: https://www.typescriptlang.org/play?target=99&ts=4.1.0-beta#code/FDDGHsDsGcBcAI4Cd4F4DkBDADtgNgKbw74HoDcw81NtNEM4hAdHuAOYAUyzSB+mUAU5ZchdABp04JJkjsC0dAEpllKvCA

Related Issues: None
Issue raised by amityadav.java@gmail.com

@tguichaoua
Copy link

@tguichaoua tguichaoua commented Nov 2, 2020

To perform a global replacement you have to use Regex: Js ref

var str = "Mr Blue has a blue house and a blue car";
var res = str.replace(/blue/gi, "red");

But yes, the IntelliSense doc is confusing.

@MartinJohns
Copy link
Contributor

@MartinJohns MartinJohns commented Nov 2, 2020

Either correct the IntelliSense doc or fix the replace function.

Only correcting the documentation is a viable option. TypeScript does not provide the string.replace function, it's part of the JavaScript engine.

@DanielRosenwasser
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Nov 3, 2020

-     * @param replaceValue A string containing the text to replace for every successful match of searchValue in this string.
+     * @param replaceValue A string containing the text to replace the first successful match of searchValue in this string.
@Jujhar Jujhar linked a pull request that will close this issue Nov 3, 2020
@Jujhar
Copy link

@Jujhar Jujhar commented Nov 3, 2020

Created a PR with updated documentation.

@jainsamyak
Copy link

@jainsamyak jainsamyak commented Nov 16, 2020

-     * @param replaceValue A string containing the text to replace for every successful match of searchValue in this string.
+     * @param replaceValue A string containing the text to replace the first successful match of searchValue in this string.

If you use a regex for replacing all occurrences, then "text to replace the first successful match of searchValue in this string" will also become confusing. I suggest that the doc clearly convey 2 options for the replaceValue param:

  1. "first successful match" in case only string in search value.
  2. "all successful matches" in case regex used as search value.
@Jujhar
Copy link

@Jujhar Jujhar commented Nov 16, 2020

* @param replaceValue A string containing either the first successful match of searchValue in case using a string in search value or all successful matches in case regex used. ?

@jainsamyak
Copy link

@jainsamyak jainsamyak commented Nov 16, 2020

More like:
A string containing the text to replace for successful matches of the searchValue in this string (replaces either first successful match of searchValue in case of using a string or every successful match in case a regex is used)

Might also use "first occurrence" in place of "first successful match".

@Jujhar
Copy link

@Jujhar Jujhar commented Nov 17, 2020

Updated it accordingly.

@sandersn sandersn added this to the Backlog milestone Nov 18, 2020
@Jujhar
Copy link

@Jujhar Jujhar commented Nov 19, 2020

I changed it to below after comments from @sandersn on PR page

* @param replaceValue A string or function to be called for some or all matches containing the text to replace from successful matches of string or RegExp searchValue. If pattern is a string, only the first occurrence is replaced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

7 participants
You can’t perform that action at this time.