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

Fix some issues in the Bash engine #459

Merged
merged 4 commits into from
Aug 1, 2024

Conversation

beatussum
Copy link
Contributor

@beatussum beatussum commented Jul 31, 2024

This pull request refers to this issue I reported some days ago.

It's not perfect but it should do the job in most of the cases.

I tried to respect your coding style. I hope I haven't made mistake and I'm waiting for review.

All tests passed.

@beatussum beatussum changed the title Fix some issue in the Bash engine Fix some issues in the Bash engine Jul 31, 2024
@SimonKagstrom
Copy link
Owner

Thanks!

You had a great example in the issue, is it possible to add that to a small test case?

Bash parsing is fairly nasty. The python support is much more slim, and easier to follow, but bash requires quite a lot of special cases. I had hopes of writing a proper parser, but with these changes (and some other), it's probably good enough as it is!

And as for the coding style, I've started to change it in some files, so right now everything is in flux and your changes look good to me :-)

@beatussum
Copy link
Contributor Author

I've dropped the commit which was supposed to fix coverage of multiline arrays. Indeed, it seems to be quiet difficult as trap <arg> DEBUG does not have the same behavior with the different syntaxes used to initialize an array:

trap 'echo ${LINENO}' DEBUG

declare -a array=( # <- this line
    a
    b
    c
)

declare array=(
    a
    b
    c
) # <- this line

declare array=(
    a
    b
    c
) # <- this line

See-also: SimonKagstrom#457
Signed-off-by: Mattéo Rossillol‑‑Laruelle <beatussum@protonmail.com>
See-also: SimonKagstrom#457
Signed-off-by: Mattéo Rossillol‑‑Laruelle <beatussum@protonmail.com>
With the last commit wich updates how subshells are parsed, the behavior (minor
change) of kcov has changed; indeed, in order to skip the ')' for subshell
functions, subshells, in general, does not have their ')' hit anymore.

See-also: a750d47
See-also: SimonKagstrom#457
Signed-off-by: Mattéo Rossillol‑‑Laruelle <beatussum@protonmail.com>
See-also: a750d47
See-also: a03bf8d
See-also: SimonKagstrom#457
Signed-off-by: Mattéo Rossillol‑‑Laruelle <beatussum@protonmail.com>
@SimonKagstrom SimonKagstrom merged commit 1fda503 into SimonKagstrom:master Aug 1, 2024
10 checks passed
@SimonKagstrom
Copy link
Owner

Merged, thanks a lot!

@beatussum beatussum deleted the bash branch August 1, 2024 16:39
beatussum added a commit to beatussum/join that referenced this pull request Aug 5, 2024
This reverts commit 7baa1a5.

Indeed, the changes added by this previous commit were due to a bug in kcov
which is now fixed.

See-also: SimonKagstrom/kcov#459
See-also: SimonKagstrom/kcov#457
Signed-off-by: Mattéo Rossillol‑‑Laruelle <beatussum@protonmail.com>
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