search-replace falls back to plain version lookup in some cases

[sanderr]

bumpversion.utils.contains is used as a guard to ensure that when the search pattern is not found, bump2version will not fall back to the default (as mentioned in the README). However, due to inconsistent matching behavior between bumpversion.utils.contains and bumpversion.utils.replace, in some cases it does fall back.

An example: Suppose version.txt contains the following (note the leading spaces):

  name = my_package
  version = 1.0.0
  name = other_package
  version = 1.0.0

An attempt at a bumpversion.cfg file to match this:

[bumpversion]
current_version = 1.0.0

[bumpversion:file:version.txt]
search = name = my_package
  version = {current_version}
replace = name = my_package
  version = {new_version}

• Expected behavior: either this bumps the first 1.0.0 and leaves the other one alone or this fails with an error message.
• Actual behavior: it replaces both occurrences of 1.0.0 with the entire replace string, resulting in the following:

  name = my_package
  version = name = my_package
  version = 1.1.0
  name = other_package
  version = name = my_package
  version = 1.1.0

The cause is that bumpversion.utils.contains uses in to check whether the first and last lines match, while bumpversion.utils.replace uses str.replace. The former matches even if the last line has leading characters not in the search line or if the first line has trailing characters not in the search line. I believe this is incorrect. This method should use == lookbehind[0].lstrip() and == lookbehind[-1].rstrip() instead of in lookbehind[0] and similar.

[florisla]

I agree, the use of in is incorrect.

Not sure if we can use == lookbehind[0].lstrip() because that would mean that extra first-line leading characters (and last-line trailing characters) are not allowed in the target file. That's a choice we have to make.

The alternative is to use lookbehind[0].endswith(search_lines[0]). Where we perhaps need to right-strip both strings.

[sanderr]

You're right, lstrip and rstrip only strip whitespace of course, I agree lookbehind[0].endswith(search_lines[0]) would be better. I don't think you'd benefit from right-stripping both strings though (for the first line specifically) as it would introduce similar inconsistencies when compared to the behavior of str.replace. If there is trailing whitespace in either the search string or the file, it should be in the other as well to produce an exact match (though I don't think you should ever intentionally have trailing whitespace anywhere).

Come to think of it, have you considered just using search in f.read()? As far as I see this would be equivalent to the behavior we're talking about here, and it would certainly be consistent with str.replace. You'd only lose some logging information.

[sanderr]

Additionally, if you'd use endswith, you'd have to consider the case where there's only one search line separately. If there's only one llne, in is correct I think.