@bcoconni do you approve this?

Currently python doesn't work as is.

Could you be more specific about that ? Because I don't see what this change is actually fixing:

• Scripts are passed via the argument --script, not --scriptfile.
• Aircraft is passed via the argument --aircraft, not --aircraftfile.

and both are reported to require a filename in the metavar= parameter.

In addition, renaming the argument --planet has a number of inconvenience:

1. The C++ code is using --planet so changing the argument only in the Python code would make the C++ and Python code inconsistent:
   

   jsbsim/src/JSBSim.cpp

   Lines 735 to 741 in 5be7eab

   	} else if (keyword == "--planet") {
   	if (n != string::npos) {
   	PlanetName = SGPath::fromLocal8Bit(value.c_str());
   	} else {
   	gripe;
   	exit(1);
   	}

   
   

   jsbsim/src/JSBSim.cpp

   Line 849 in 5be7eab

   cout << " --planet=<filename> specifies a planet definition file" << endl;

2. The commit 853334f that introduced the argument --planet is now 2 years old meaning that it is part of the releases 1.2.0 and 1.2.1. Changing the argument name would break backward compatibility.

So I'd suggest not to approve this PR, unless a compelling reason to do so is given.

Happy to elaborate, I expect I'm running into this as someone coming at it from a pure python scripting perspective, so might have slipped through the testing cracks. But either way no way of running in python script mode currently works.

1. pip install jsbsim
2. JSBSim.py

You'll get the error:

Traceback (most recent call last):
  File "/Users/tom.wainwright/Documents/cases/DEMO/JSBSim/jsb_env/bin/JSBSim.py", line 184, in <module>
    if args.planetfile:
       ^^^^^^^^^^^^^^^

Alternative change is to change L184 to if args.planet: But either way the argparse name, and the reference to it need to match up. I wasn't aware there was convention in other bits of the code so happy to just change L184 too.

Alternative change is to change L184 to if args.planet: But either way the argparse name, and the reference to it need to match up. I wasn't aware there was convention in other bits of the code so happy to just change L184 too.

Thanks for further explanations. Good catch ! So yes, please replace L184 as you suggested.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 24.96%. Comparing base (5be7eab) to head (df05f6a).
Report is 7 commits behind head on master.

@@           Coverage Diff           @@
##           master    #1215   +/-   ##
=======================================
  Coverage   24.96%   24.96%           
=======================================
  Files         170      170           
  Lines       19281    19281           
=======================================
  Hits         4814     4814           
  Misses      14467    14467           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

The PR has been merged.
Thanks for your contribution !