Skip to content
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

fix style of last argument of argument lists #341

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file modified examples/isotp_send.py
100755 → 100644
Empty file.
30 changes: 10 additions & 20 deletions examples/mksomersaultmodifiedpdx.py
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,9 @@ def find_named_object(item_list: List[T], name: str) -> T:
byte_position=0,
coded_value=FLIC_FLAC_SID,
bit_position=None,
sdgs=[],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep the comma always, it makes reading diffs easier

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

automating the implementation of this is pretty difficult, because for trivial stuff like f(x) we probably don't want the trailing comma. (and I really don't want to do this fully manually.) That said, I'm also happy to close this PR unmerged and keep the current state of the art...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is if the list or function arguments are multi-line, a trailing comma helps better with diffs

A trailing comma is not needed for a one line list

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. I agree that it makes sense, but my point was that writing a script for "semi-automatic" refactoring for this style is pretty hard... (for the current PR I could basically get away with a bit of regex replacement magic, but I don't see a way to get around a "bracket+string" parser if I have to count the number of arguments, etc.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

under these constraints, shall we close or merge?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just close

)
sdgs=[])
]),
byte_size=None,
)
byte_size=None)
somersault_young_dlr.requests.append(flic_flac_request)

flic_flac_positive_response = Response(
Expand All @@ -145,8 +143,7 @@ def find_named_object(item_list: List[T], name: str) -> T:
byte_position=0,
coded_value=uds.positive_response_id(FLIC_FLAC_SID),
bit_position=None,
sdgs=[],
),
sdgs=[]),
ValueParameter(
oid=None,
short_name="can_do_backward_flips",
Expand All @@ -158,11 +155,9 @@ def find_named_object(item_list: List[T], name: str) -> T:
dop_ref=OdxLinkRef("somersault.DOP.boolean", doc_frags),
dop_snref=None,
bit_position=None,
sdgs=[],
),
sdgs=[])
]),
byte_size=None,
)
byte_size=None)
somersault_young_dlr.positive_responses.append(flic_flac_positive_response)

flic_flac_service = DiagService(
Expand All @@ -189,14 +184,11 @@ def find_named_object(item_list: List[T], name: str) -> T:
state_transition_refs=[],
semantic="FUNCTION",
request_ref=OdxLinkRef.from_id(flic_flac_request.odx_id),
pos_response_refs=[
OdxLinkRef.from_id(flic_flac_positive_response.odx_id),
],
pos_response_refs=[OdxLinkRef.from_id(flic_flac_positive_response.odx_id)],
neg_response_refs=[
OdxLinkRef.from_id(somersaultecu.somersault_negative_responses["general"].odx_id),
OdxLinkRef.from_id(somersaultecu.somersault_negative_responses["general"].odx_id)
],
sdgs=[],
)
sdgs=[])

# create a new list of diagnostic communications that does not include
# the "set_operation_params" and "compulsory_program" services
Expand Down Expand Up @@ -225,14 +217,12 @@ def find_named_object(item_list: List[T], name: str) -> T:
"The modified PDX file is primarily intended to be used a demo for the ",
"'compare' command line tool."
]),
formatter_class=argparse.RawTextHelpFormatter,
)
formatter_class=argparse.RawTextHelpFormatter)

argparser.add_argument(
"output_pdx_file",
metavar="OUTPUT_PDX_FILE",
help="Path to the where the resulting .pdx file is written",
)
help="Path to the where the resulting .pdx file is written")

args = argparser.parse_args()

Expand Down
6 changes: 2 additions & 4 deletions examples/mksomersaultpdx.py
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@
"automotive diagnostic services in python and making them interact",
"with existing tooling.",
]),
formatter_class=argparse.RawTextHelpFormatter,
)
formatter_class=argparse.RawTextHelpFormatter)

argparser.add_argument(
"output_pdx_file",
metavar="OUTPUT_PDX_FILE",
help="Path to the where the resulting .pdx file is written",
)
help="Path to the where the resulting .pdx file is written")

args = argparser.parse_args()

Expand Down
6 changes: 2 additions & 4 deletions examples/pdxcopy.py
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,14 @@
"PDX files using odxtools, but it can also be used to strip",
"'unnecessary' auxiliary data from the input PDX file.",
]),
formatter_class=argparse.RawTextHelpFormatter,
)
formatter_class=argparse.RawTextHelpFormatter)

argparser.add_argument(
"input_pdx_file", metavar="INPUT_PDX_FILE", help="Path to the input .pdx file")
argparser.add_argument(
"output_pdx_file",
metavar="OUTPUT_PDX_FILE",
help="Path to the where the resulting .pdx file is written",
)
help="Path to the where the resulting .pdx file is written")

args = argparser.parse_args()

Expand Down
Loading
Loading