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

Adding CaseSensitive parameter to StrReplace. #103

Merged
merged 1 commit into from
Jul 11, 2020

Conversation

HelgeffegleH
Copy link
Contributor

Hello 👋

New:

ReplacedStr := StrReplace(Haystack, SearchText [, ReplaceText, CaseSensitive := false, OutputVarCount, Limit := -1])

Implementation is copied from BIF_InStr.

Reason for change: convenience.

New:

ReplacedStr := StrReplace(Haystack, SearchText [, ReplaceText, CaseSensitive := false, OutputVarCount, Limit := -1])

Implementation is copied from BIF_InStr.

Reason for change: convenience.
@Lexikos
Copy link
Collaborator

Lexikos commented Jun 23, 2018

This seems like it could be useful, but I wonder: how often would it be used, and how often are the OutputVarCount and Limit parameters used?

It is already possible to control the case-sensitivity of StrReplace (though perhaps not in the most convenient way), whereas OutputVarCount and Limit provide functionality that would otherwise require either a custom replacement function or RegExReplace. The current parameter order is identical to RegExReplace, which aides memorization.

I couldn't find any instance of StrReplace with more than three parameters in any of my scripts, but I know I have used OutputVarCount (or at least ErrorLevel with StringReplace). I do not recall having ever used StringCaseSense with StrReplace/StringReplace, although that might be because I am comfortable with regex. (I'm not sure that I've used StringCaseSense at all.)

@HelgeffegleH
Copy link
Contributor Author

Hello.

I wonder: how often would it be used

In one sense it will always be used, that is, since not specifying the parameter is equivalent to passing false. I think it is very common that people do not set stringcasesense 'off', even when this is the desired setting, just because this is default unless changed in auto-exec. section, that is a mistake for a general/shared function. Further, I would probably often specify true to avoid using the (much slower) locale obeying function when replacing something like ',' with '|', where neither locale nor case matters. That would probably be a rarely significant optimisation though.

I do not recall having ever used StringCaseSense with StrReplace/StringReplace

I think stringcasesense is rarely used with strreplace, because, either the code author doesn't realise that the replacement will be case sensitive if used in a script where stringcasesense is 'on', that is, it is happy with the default stringcasesense 'off', or the author is aware and then uses regexreplace with or without the i option, since it is more succinct than setting and restoring stringcasesense. Personally, I use regexreplace sometimes just because of this, and sometimes I forget or just do not care about the implications of stringcasesense and use strreplace. Some of my code most certainly will not work well if put in a script where stringcasesense 'on' has been set in the auto-exec. section.

Regarding the parameter order, I'm quite neutral to it. I made a guess that the new parameter would be specified more often than the others. Whenever I use the OutputVarCount or Limit parameters for either regexreplace or strreplace, I need to check the docs for the order anyways.

Cheers.

@jeeswg
Copy link
Contributor

jeeswg commented Aug 3, 2019

I would add StartPos (consistent with RegExReplace) and CaseSensitive parameters:
StrReplace() should have option to replace from rear of string
Or possibly turn Limit into Options (to cover: Limit, CaseSensitive, and maybe StartPos).

(I occasionally find Limit useful for restricting replacements to the first instance.)
(I regularly use Count to count the occurrences of a string. Although would prefer a StrCount function for this. And sometimes use it for 'replace and count'.)

@jeeswg
Copy link
Contributor

jeeswg commented Feb 3, 2020

Here's a possible solution for StrReplace.

Parameters

1 Haystack
2 SearchText
3 ReplaceText
4 (change) before: OutputVarCount, after: CaseSensitive
5 Limit
6 (new) StartingPos
7 (new) OutputVarCount

(and a StrCount function)

Proposals

modify StrReplace (before/after) (2 new params, 1 swap):
ReplacedStr := StrReplace(Haystack, SearchText [, ReplaceText, OutputVarCount, Limit := -1])
ReplacedStr := StrReplace(Haystack, SearchText [, ReplaceText, CaseSensitive := false, Limit := -1, StartingPos := 1, OutputVarCount := ""])

introduce StrCount (AHK v1/v2) (params match InStr, although StartingPos could be omitted until fully discussed/scrutinised):
Count := StrCount(Haystack, SearchText [, CaseSensitive := false, StartingPos := 1])

Rationale

  • only 1 param modified
  • the first 6 params (except param 4) match RegExReplace (for easy transfer)
  • case insensitive by default (like InStr) (more intuitive/secure, and less verbose, than using StringCaseSense)

for reference (InStr/RegExReplace):
FoundPos := InStr(Haystack, Needle [, CaseSensitive := false, StartingPos := 1, Occurrence := 1])
NewStr := RegExReplace(Haystack, NeedleRegEx [, Replacement := "", OutputVarCount := "", Limit := -1, StartingPosition := 1])

I wrote the 2 functions as custom AHK functions and they neatly solved all my replace/count/StringCaseSense woes.
I was able to combine multiple separate custom functions into one StrReplace function.
Writing new code is easier and more intuitive.

Conversion cost re. the Count parameter

The conversion cost was very low:

[I had 73 of these]
e.g. modify old code (before/after: count and replace) (add 3 commas):
vText := StrReplace(vText, vBefore, vAfter, vCount)
vText := StrReplace(vText, vBefore, vAfter,,,, vCount)

[I had 40 of these]
e.g. modify old code (before/after: count only):
StrReplace(vHaystack, vNeedle, "", vCount)
vCount := StrCount(vHaystack, vNeedle)

[I had 17 of these]
e.g. modify old code (before/after: count and increment):
StrReplace(vHaystack, vNeedle, "", vCount), vCount++
vCount := StrCount(vHaystack, vNeedle) + 1

That's of around 1774 StrReplace instances.

@Lexikos Lexikos merged commit 81b7006 into AutoHotkey:alpha Jul 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants