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

Minor fixes around annotations #305

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

Minor fixes around annotations #305

wants to merge 2 commits into from

Conversation

nvasilakis
Copy link
Collaborator

No description provided.

@nvasilakis nvasilakis requested a review from angelhof July 29, 2021 16:21
@github-actions
Copy link

OS:ubuntu-18.04
Thu Jul 29 16:33:48 UTC 2021
intro: 2/2 tests passed.
interface: 6/6 tests passed.
compiler: 48/54 tests passed.
evaluation/tests/results/ann-agg_2_distr_.time
evaluation/tests/results/ann-agg_8_distr_.time
evaluation/tests/results/bigrams_2_distr_.time
evaluation/tests/results/bigrams_8_distr_.time
evaluation/tests/results/shortest_scripts_2_distr_.time
evaluation/tests/results/shortest_scripts_8_distr_.time

@nvasilakis nvasilakis changed the title Fix cut annotation Minor fixes around annotations Jul 29, 2021
@github-actions
Copy link

OS:ubuntu-20.04
Thu Jul 29 16:53:02 UTC 2021
intro: 2/2 tests passed.
interface: 6/6 tests passed.
compiler: 48/54 tests passed.
evaluation/tests/results/ann-agg_2_distr_.time
evaluation/tests/results/ann-agg_8_distr_.time
evaluation/tests/results/bigrams_2_distr_.time
evaluation/tests/results/bigrams_8_distr_.time
evaluation/tests/results/shortest_scripts_2_distr_.time
evaluation/tests/results/shortest_scripts_8_distr_.time

@angelhof
Copy link
Member

I think that we should not merge this before we investigate what are the new failing test cases. I might have some time to take a look. If we want to merge these (and other changes) faster (without making sure that correctness tests pass) we can use the future branch.

@nvasilakis
Copy link
Collaborator Author

@angelhof Agreed! BTW we think we know why tests are failing, and we're working to fix them.

"stdin-hyphen",
"empty-args-stdin"
],
"short-long": [
Copy link
Member

Choose a reason for hiding this comment

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

This is not yet supported by PaSh. We can open an issue or add a test with a long version of one of these flags and use that in a PR to implement that.

],
"options": [
"stdin-hyphen",
"empty-args-stdin"
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, this is not implemented at the moment (and is very crucial for all the commands that read from stdin if they have no arguments). In its current form, the annotation leads to test failures because uses of cut that read from stdin are not supported (that is why previously cut's annotation only reads from stdin).

Copy link
Member

Choose a reason for hiding this comment

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

This is probably the reason why tests fail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK there are probably multiple reasons why the tests are failing:P

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