Skip to content

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
"'}'",
[
[
"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