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.

Sign up for GitHub

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

Escape backslashes in search and replacement #592

Draft
wants to merge 7 commits into
base: staging
Choose a base branch
from
+44 −7
Draft

Escape backslashes in search and replacement #592

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
580835a
Fix typos
lens0021 Nov 14, 2024
eaca70d
Escape backslashes in search and replacement
lens0021 Nov 14, 2024
287e64f
Add Bash <=5.1 compatibility
lens0021 Dec 12, 2024
1537ee0
docs: Add a TODO
lens0021 Dec 12, 2024
3f610a8
Update src/std/text.ab
lens0021 Dec 16, 2024
a2c6012
Import bash_version()
lens0021 Dec 16, 2024
32e30b0
Fix < 5.1 and add tests
lens0021 Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions src/std/text.ab
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,26 @@
/// Replaces all occurences of a pattern in the content with the provided replace text.
/// Replaces all occurrences of a pattern in the content with the provided replace text.
#[allow_absurd_cast]
pub fun replace(source, search, replace) {
return "\$\{source//{search}/{replace}}"
search = trust $ echo \$\{{nameof search}//\\\\/\\\\\\\\} $
// TODO: Revisit after https://github.com/amber-lang/amber/issues/632
const bash_major = trust $ echo \"\$\{BASH_VERSINFO[0]}\" $ as Num
lens0021 marked this conversation as resolved.
Show resolved Hide resolved
const bash_minor = trust $ echo \"\$\{BASH_VERSINFO[1]}\" $ as Num
hdwalters marked this conversation as resolved.
Show resolved Hide resolved
if bash_major > 5 or (bash_major == 5 and bash_minor > 1) {
replace = trust $ echo \$\{{nameof replace}//\\\\/\\\\\\\\} $
} else {
replace = trust $ echo \$\{{nameof replace}//\\\\/\\\\} $
}
return trust $ echo \$\{{nameof source}//{search}/{replace}} $
Copy link
Contributor Author

@lens0021 lens0021 Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I know this is dirty. When #622 is resolved, this could be improved.

}
lens0021 marked this conversation as resolved.
Show resolved Hide resolved

/// Replaces the first occurence of a pattern in the content with the provided replace text.
/// Replaces the first occurrence of a pattern in the content with the provided replace text.
pub fun replace_once(source, search, replace) {
return "\$\{source/{search}/{replace}}"
search = replace(search, "\\", "\\\\")
replace = replace(replace, "\\", "\\\\")
return trust $ echo \$\{{nameof source}/{search}/{replace}} $
}
lens0021 marked this conversation as resolved.
Show resolved Hide resolved

/// Replaces all occurences of a regex pattern in the content with the provided replace text.
/// Replaces all occurrences of a regex pattern in the content with the provided replace text.
///
/// Function uses `sed`
pub fun replace_regex(source: Text, search: Text, replace: Text, extended: Bool = false): Text {
Expand Down Expand Up @@ -45,7 +57,7 @@ pub fun words(text: Text): [Text] {
return split(text, " ")
}

/// Merges text using the delimeter specified.
/// Merges text using the delimiter specified.
pub fun join(list: [Text], delimiter: Text): Text {
return trust $ IFS="{delimiter}" ; echo "\$\{{nameof list}[*]}" $
}
Expand Down
4 changes: 4 additions & 0 deletions src/tests/stdlib/replace.ab
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import * from "std/text"

// Output
// apple apple
// apple
// banana\\

main {
echo replace("banana banana", "banana", "apple")
echo replace("\\", "\\", "apple")
Copy link
Contributor

Choose a reason for hiding this comment

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

To confirm that replace replaces multiple instances, may I suggest testing multiple occurrences of the search text:

echo replace("\\ \\", "\\", "apple")
echo replace("\\banana\\", "\\", "\\\\")

Copy link
Contributor

Choose a reason for hiding this comment

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

You might also consider cloning the modified test, and overwriting replace_once.ab, with adjusted expected output.

Copy link
Contributor Author

@lens0021 lens0021 Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Added tests for replace_once and edited text.ab to make the test pass, though I did not know why it was broken.

Copy link
Contributor

@hdwalters hdwalters Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm still not entirely happy with these tests:

  1. I had in mind that both test files would contain calls to replace and replace_once, with identical parameters, to show the differences in behaviour.
  2. Each function call should test a specific use case; they don't all have to contain backslashes!
  3. Each function call should contain multiple copies of the search text.

Copy link
Contributor

@hdwalters hdwalters Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

For example:

import * from "std/text"

// Output
// TWO TWO TWO
// a\\b\\c\\d
// first..second..third..fourth

main {
    echo replace("one one one", "one", "TWO")
    echo replace("a\\b\\c\\d", "\\", "\\\\")
    echo replace("first\nsecond\nthird\nfourth", "\n", "..")
}

echo replace("banana\\", "\\", "\\\\")
}
2 changes: 1 addition & 1 deletion src/tests/stdlib/replace_once.ab
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * from "std/text"
main {
echo replace_once("Succinctly", "inctly", "eeded")
echo replace_once("Succinctly\\", "inctly\\", "eeded")
Copy link
Contributor

@hdwalters hdwalters Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

For example:

import * from "std/text"

// Output
// TWO one one
// a\\b\c\d
// first..second
// third
// fourth

main {
    echo replace_once("one one one", "one", "TWO")
    echo replace_once("a\\b\\c\\d", "\\", "\\\\")
    echo replace_once("first\nsecond\nthird\nfourth", "\n", "..")
}

}
Loading