[osh] Add support for shopt -s nocasematch #1748
Conversation
All tests in spec/nocasematch-match/test.sh now pass (3 failures previously allowed). Still need to do the C++ translation.
It appears Optional[T] roughly means Union[T, NoneTyp]
Changed the interface, but didn't implement it
There was a problem hiding this comment.
This is excellent! Thank you
Only thing is that I think we should wrap the constants
BTW I fixed the C++ build, and you can look at the diff to see what happened. It was pretty easy!
Also I forgot to mention that the type checking is done with
devtools/types.sh check-oils
which I documented here!
/~https://github.com/oilshell/oil/wiki/Oil-Dev-Cheat-Sheet
(Melvin and Aidan have been getting by without this, but I welcome updates to the dev docs!)
osh/cmd_eval.py
Outdated
| @@ -111,6 +111,9 @@ | |||
| RaiseControlFlow = 1 << 1 # eval/source builtins | |||
| Optimize = 1 << 2 | |||
|
|
|||
| # flags for fnmatch | |||
| FnmCasefold = 1 << 4 | |||
There was a problem hiding this comment.
I think we should wrap this constant value in Python, which will make the C++ translation easier:
Example here:
/~https://github.com/oilshell/oil/blob/master/pyext/posixmodule.c#L2566
and then we can do
from libc import FNM_CASEFOLD
osh/sh_expr_eval.py
Outdated
| @@ -80,6 +80,10 @@ | |||
| # (( val = a[key] )) # osh/sh_expr_eval.py:509 (Id.Arith_LBracket) | |||
| # | |||
|
|
|||
| # flags for libc functions | |||
| FnmCasefold = 1 << 4 | |||
| RegIcase = 1 << 1 | |||
There was a problem hiding this comment.
Ditto here, let's create a constant that we can import like this
from libc import REG_ICASE (or whatever its spelling is)
BTW when I fiddle with pyext/, I also use pyext/libc_test.py, in case that's helpful
osh/sh_expr_eval.py
Outdated
| @@ -1042,14 +1046,17 @@ def EvalB(self, node): | |||
| raise AssertionError(op_id) # should never happen | |||
|
|
|||
| if arg_type == bool_arg_type_e.Str: | |||
| eval_flags = 0 | |||
| if self.exec_opts.nocasematch(): | |||
| eval_flags |= FnmCasefold | |||
There was a problem hiding this comment.
how about calling these fnmatch_flags and regex_flags ?
Also in this case we can probably use a ternary expression
fnmatch_flags = FNM_CASEFOLD if self.exec_opts.nocasematch() else 0
|
Hi Ellen, this looks like it's very close, and could go in the next release? Let me know if you have any questions / issues Thanks again for the contribution! |
|
Hi Andy, no questions at this point I don't think. (Though I do need to remind myself of the detail.) Just been one of those weeks. What's the time frame to get these changes into the release? |
Rather than define C constants like FNM_CASEFOLD in the Python modules, make them available from pyext/libc. Refactor flag variable names and replace if statements with ternary operators where there are two outcomes.
|
I've pushed up the refactoring, which has caused more C++ errors. Don't currently understand why the consts aren't in libc, but thinking it's to do with Thoughts on the refactoring itself:
|
We should move away from this style with mycpp/yaks "modules"
|
Great thank you! I added some stuff to Yeah Python's (In our own C++ code we would use DCHECK(), but meh ) |
All tests in spec/nocasematch-match/test.sh now pass (3 failures previously allowed).
Still need to do the C++ translation.
Fixes #701