-
Notifications
You must be signed in to change notification settings - Fork 192
String list new #680
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
String list new #680
Conversation
The API for the module to manipulate lists of strings has been discussed and this has resulted in the current implementation
There was a typo in some comments - corrected
Document the linked_list type, including examples, and correct a few minor mistakes in the code.
Correct the subdirectory for the examples in the documentation.
Move the actual implementations of the linked list type (the linked list itself and the child list component) to the directory src.
Use an include statement instead of relying on the automatic module identification to get access to the print_list subroutine. As it is merely an auxiliary, it should not become part of the "official" source code.
Use an internal routine for print_list instead of an included module, as the buil process still wants to find the module's source.
Add the subdirectory "linked_list" to the overall CMake file and add a specific CMake file for the examples therein.
Rename the examples so that the names of the targets do not conflict with existing examples for other modules (notably in the strings subdirectory
Use a dedicated macro to make sure that the include directory is registered per target. It did not show up in the earlier attempt.
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.
I guess that this file should be removed from the directory.
I think the fpm-deployment CI is failing because you need to update the ci/fpm-deployment.sh
script to also copy over the included files into an ./include
folder in the root directory.
Correct the CMakeLists.txt file in the src directory to include the linked list modules , also adjust the fpm-deployment.sh to copy any auxiliary source files from the example directory.
Rename the include file to "*.inc" so that it is no longer recognised by the build system. Of course, the example source files had to be adapted as well.
Correct the performance test program - apparently it was using some deprecated names and it was not built using the CMake build system.
Thanks for the comments. I was not sure how I could introduce an include directory, so instead I simply copy the extra file in to the example directory. I had to rename the file to something that is not confused with a separate Fortran program. Also I had to tweak the CMakeLists.txt files for the test program. I seem to be almost there, but the test program fails to run properly. Looking into that now.
Update the test program to NOT read from stdin - that does not work in the CI build and execute environment.
The test program was failing because it tried to read from the keyboard. Changed it to simply use a length of one million elements.
Create the missing CMake file for the test program.
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.
Thank you for this progress @arjenmarkus . Here are some suggestions.
Note that we use the suffix _type
for derived types in stdlib
.
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.
What is the difference with the stdlib
stringlist_type
?
Updated the documentation after review by jvdp1. Adjusted the type name (suffix _type) and applied several cosmetic changes. Also added a more extensive test program and fixed several bugs.
As the type is now "linked_list_type", the examples had to be adjusted. There turned out to be a bug regarding the intent for the get() function. This has been repaired.
... into linked-list-new
The file "linked_list_aux.inc" was not found - fpm test.
Something went wrong with the included routine in the examples. So instead, simply copy the source code for it in all examples.
Two failures: on MSYS2 CMake was not found, on Ubuntu latest ifort was not found. How should we resolve these problems?
Such issues should have been solved with one of the latest PR. The CI seem to be ok now.
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.
could the string_type
present in stdlib_string_type
be used instead of redefining a new one?
No description provided.