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

Remove double quotation from parameter #122

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ishaangandhi
Copy link
Contributor

@ishaangandhi ishaangandhi commented Oct 9, 2020

Right now, an extra set of double quotes surrounds every parameter. This pull request fixes that.

Test plan:

$ make check
if command -v ocamlopt >/dev/null; then cp src/c/dune.native src/c/dune; fi
dune build @install
Done: 0/0 (jobs: 0)[ -e bin ] || ln -sf _build/install/default/bin bin
[ -e lib ] || ln -sf _build/install/default/lib/morbig lib
* Testsuite configuration
Using local ~/morbig/bin/morbig.
* Summary:
-----------------------------------
| Tests | Passed | Failed | Total |
|-------|--------|--------|-------|
| good  |    110 |      0 |   110 | 
| bad   |     67 |      0 |    67 |
| all   |    177 |      0 |   177 |
-----------------------------------

         ----------------
         Congratulations!
         ----------------

@Niols Niols changed the base branch from master to main March 31, 2023 14:37
@yurug yurug force-pushed the double-quote-bug branch from 4786efa to d5edea9 Compare May 10, 2023 07:24
@@ -483,7 +483,7 @@
"'}'",
[
[
"WordDoubleQuoted",
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this one is correct because now we have a WordSingleQuoted inside another WordSingleQuoted. I think that, similarly to the other cases, we should just remove the layer of WordDoubleQuoted altogether. Not entirely sure though.

@Niols
Copy link
Member

Niols commented May 10, 2023

The tests suite in CI is currently unreadable. This should be fixed by #169 so maybe we should wait for that, rebase this on top of main and then see?

@yurug
Copy link
Contributor

yurug commented May 10, 2023

The tests suite in CI is currently unreadable. This should be fixed by #169 so maybe we should wait for that, rebase this on top of main and then see?

Yes, let's do that.

@Niols Niols force-pushed the double-quote-bug branch from d5edea9 to 0dbb64c Compare May 12, 2023 15:02
@Niols
Copy link
Member

Niols commented May 12, 2023

@yurug Called for a rebase using #169. Now the CI should give us much more readable outputs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants