-
Notifications
You must be signed in to change notification settings - Fork 168
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
Implement strip and chomp as supplement to trim #343
Conversation
There are probably use cases for both. Could we design
But that unfortunately complicates the API-- |
We would have to force keyword arguments by some hacks, it's not impossible, but quite ugly in Fortran... function chomp_set(string, force_kwargs, set)
character(len=*), intent(in) :: string
! dummy type to force key-value argument, make sure signature is different from substring variant
type :: force_kwargs_type
class(*), private, allocatable :: set
end type force_kwargs_type
type(force_kwargs_type), intent(in), optional :: force_kwargs
character(len=*), intent(in) :: set
end function
function chomp_substring(string, force_kwargs, substring)
character(len=*), intent(in) :: string
! dummy type to force key-value argument, make sure signature is different from set variant
type :: force_kwargs_type
class(*), private, allocatable :: substring(:)
end type force_kwargs_type
type(force_kwargs_type), intent(in), optional :: force_kwargs
character(len=*), intent(in) :: substring
end function |
I don't think that hack works. Since the "dummy" argument is optional in both cases, the interfaces are ambiguous. |
Yeah, you're right, this interface doesn't work, that's why I put it up as puzzle at the discourse. Maybe there is a way to make it work in Fortran? |
- empty set array, [character(len=1)::], does missmatch due to string length of zero (!) to the actual dummy set argument
Thanks for feedback so far. With two approvals I will merge later this week unless there is more to discuss on this proposal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments:
- Do we want examples in the spec for a
type(string_type)
variable or is this supposed to be self-explanatory? - I found the name of the test driver
string_trim
a bit confusing given that the functions tested are calledstrip
andchomp
. My only alternative would bestring_strip_chomp
. - I wasn't sure what was the purpose of testing the functions with and without the keyword argument, e.g:
call check(chomp(string_type("hello"), "lo") == "hel") call check(chomp(string_type("hello"), substring="lo") == "hel")
- I noticed you arranged the functions in "opposite" order of how they interact, in other words functions never call functions above them. Is there some underlying reason for this?
But otherwise, I think it is good to go!
The functions are a great example of how versatile verify
is. On the other hand the high ratio of boiler-plate code for interfaces vs actual code doing stuff is somewhat disheartening property of Fortran.
Using
Mainly to pin the API down in the test, thinking about this, there is no “test” to check whether the first argument can be accessed by
Mainly the order I wrote them, no particular reason on ordering them this way.
It is indeed quite verbose, but the wrappers for the implementation are too different to create them from a fypp macro. Since the wrappers are quite similar in general we might come up with some common skeleton for all of them as the |
Thanks for the answers. With 3 approvals it is more than ready to be merged. 👍 |
I'm not convinced this is the right approach. String handling is really the realm of scripting languages and I think that the way Tcl handles these issues is better. Strip: why not trim, trimleft and trimright? Chomp: in over 30 years of writing scripts (cross platform) I've never needed such a function and Tcl doesn't have one - and the examples just don't seem real. I'd like to understand what real problem is being solved with this function. The name is terrible as well - reminds me of Perl to which I seem to be allergic! |
Originally proposed by @zbeekman in #69 (comment):
Changes to the original proposal
chomp
takes also a set which is provided as array of length one characters and the default chomp uses thewhitespace
set fromstdlib_ascii
rather than removing only newlines.