-
Notifications
You must be signed in to change notification settings - Fork 343
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
Add option to create geometric bodies (spheres, prisms, ellipsoids, etc) and compute their effect. #4583
Conversation
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.
You cannot break the +modifier syntax since we will auto-compute these from long-forms, eventually. Things like -M+bell is not a modifier since bell is a word. It has to be, e.g., -M+sbell where +s means shape. We can then later handle -Mshape=bell in long-form and auto-convert to -M+sbell. So need to be +scylinder (see typo), +scone, +sprism, etc.
Type Alaternative.
OK, -M+ssphere it is. |
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.
You need to merge master into this first, no. Otherwise, I see you are reverting several things in master related to recent bug fixes, e.g., in gmt_io.c, grdcut.c, grdedit.c.
Your test script is missing the **+**s modifier.
The listing of bell,height, etc should be italics not bold since these are parameters, and you have several references to instead of height in that section (i.e., there are lots of these). Pirimid -> Pyramid.
I had merge master but had conflicts and ended up no committing all master changes in this branch. Had also forgotten to commit the cube_mag.sh test update to +s. |
Ping |
Think you have to do all those Mimi commits |
Mini |
Ping @joa-quim
|
They are committed. |
OK, I guess you did not use the buttons we set up for you to click commit as they all show up there, so harder to see if they were fixed. Also, per @seisman the script does not have executable permission so please fix that so we can approve. |
The script has the |
OK, will fix if needed. |
FYI, I added the |
@joa-quim This PR introduces some compiler warnings:
|
Tested for a couple of them (bodies) but not all.