pcdshub / pytmc

Generate EPICS IOCs and records from TwinCAT projects - along with many TwinCAT project tools
https://pcdshub.github.io/pytmc/
Other
10 stars 11 forks source link

ENH: support concept of "extras" in stcmd generation #93

Closed klauer closed 4 years ago

klauer commented 5 years ago

Support additional template arguments from the command-line, either through directly specified VAR=VALUE, from a json string, or from a json file.

A thing I noticed about keeping only one syntax for the VAR=VALUE, but allowing it to be specified multiple times. This would force user-specified extras to always be lists instead of scalar values. As a workaround, I added the alternate syntax of := which makes it scalar.

Current functionality overrides automatically generated arguments. This may or may not be a bad thing - perhaps these settings should be relegated into template_args['argparse_args'] = {'VAR': 'VALUE'} or something similar.

I'm open to suggestions about any/all of the above.

codecov-io commented 5 years ago

Codecov Report

Merging #93 into master will decrease coverage by 1.03%. The diff coverage is 29.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
- Coverage   76.45%   75.42%   -1.04%     
==========================================
  Files          14       14              
  Lines        1508     1542      +34     
==========================================
+ Hits         1153     1163      +10     
- Misses        355      379      +24
Impacted Files Coverage Δ
pytmc/bin/stcmd.py 51.85% <29.41%> (-7.56%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7068f5d...a8835ac. Read the comment docs.

ZLLentz commented 5 years ago

Concept seems good, but I'm confused by the CLI syntax. My understanding of the code as-written is that I do the following if I want a list: --extra my_list=some --extra my_list=text -> ['some', 'text'] Or the following if I want a list of one element: --extra my_elem=ent -> ['ent'] Or the following if I want a single value: --extra my_value:=worth -> 'worth'

... But of course, these all have to be strings? Since we're coming from CLI and substituting into a st.cmd file anyway, do we need lists at all? Can't we just have a super simple --extra my_text=something interface?

If we do need lists, why two separate operators (where one is hard to use due to repeated invocation of --extra to get a list) instead of something like: --extra my_list=[some,text] --extra my_elem=[ent] --extra my_value=worth

klauer commented 5 years ago

I think

--extra my_list=[some,text]
--extra my_elem=[ent]
--extra my_value=worth

is exactly what this should have been - thanks, @ZLLentz! And perhaps renamed macro just for consistency with other tools. I'll change this up today.

klauer commented 5 years ago

For the record: I still haven't gotten around to redoing this - this PR should not be merged.