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

elprop doesn't run all methods #99

Open
fpdotmonkey opened this issue Jan 9, 2025 · 4 comments
Open

elprop doesn't run all methods #99

fpdotmonkey opened this issue Jan 9, 2025 · 4 comments

Comments

@fpdotmonkey
Copy link
Contributor

Hey, seems like a neat project and I wanted to contribute. I want to check my changes with elprop, but I seem to be misunderstanding what's meant by "call elprop/elprop with a regex matching the name of some rune lisp functions". I'm running this:

rune/elprop$ ./elprop 'upcase|downcase'

and getting this out

Generating Functions

====================
Testing: upcase-word
upcase-word: Passed

====================
Testing: downcase-word
downcase-word: Passed

All tests passed

It's running these *-word functions, but not against upcase or downcase themselves, which are certainly implemented and I'm able to call them in the repl. Am I writing the regex wrong? Is there some further incantation I need to make?

@fpdotmonkey fpdotmonkey changed the title elprop fails to run elprop doesn't run all methods Jan 9, 2025
@CeleritasCelery
Copy link
Owner

hmm, it seems to work for me. If I add this dummy function to casefiddle.rs

#[defun]
fn downcase(s: &str) -> String {
    String::from("")
}

and then run elprop with ./elprop 'upcase|downcase'

I get this output.

Generating Functions

====================
Testing: upcase
Status: Failed
Input: (upcase "𐵰")
Output: Emacs: "𐵰", Rune: "𐵐"

====================
Testing: downcase
Status: Failed
Input: (downcase " ")
Output: Emacs: " ", Rune: ""

====================
Testing: upcase-word
upcase-word: Passed

If you push your change to your fork I can take a look there. If you look at rune/target/elprop/functions.json you will see a list of all the functions it found.

Also be aware that rust and Emacs use different unicode versions so you may still get failures with a correct implementation, and that is okay.

@fpdotmonkey
Copy link
Contributor Author

My changes are up at https://github.com/fpdotmonkey/rune/tree/impl-casefiddle. Still the same results as above. I checked with ./elprop '.*' and the functions.json didn't have any of the methods I defined besides the *-word ones. I wonder if it's because the methods I define return a Result instead of a simple Object?

@CeleritasCelery
Copy link
Owner

CeleritasCelery commented Jan 9, 2025

The issue was that you returned a TypeError instead of Error. That is a valid thing to do, but the elprop code did not handle this correctly. I pushed a fix.

Also looking at your code, you define the incoming type as Object, but only string and char's are valid. This means that most of the property testing will be wasted on invalid types. To get better results, you can either use the the elprop macro or you can define a StringOrChar type similar to StringOrSymbol and then add that to elprop here.

@CeleritasCelery
Copy link
Owner

For example, you could use this to test only strings and chars:

#[elprop(String | char)]

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

No branches or pull requests

2 participants