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

[MISC] Allow recursive subcommands #233

Merged
merged 1 commit into from
Sep 23, 2024
Merged

Conversation

eseiler
Copy link
Member

@eseiler eseiler commented Feb 8, 2024

@seqan-actions seqan-actions added lint [INTERNAL] signals that clang-format ran and removed lint [INTERNAL] signals that clang-format ran labels Feb 8, 2024
@mr-c
Copy link

mr-c commented Feb 8, 2024

What does this do to the CWL export?

Copy link

codecov bot commented Feb 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.12%. Comparing base (7534545) to head (ef1d6f5).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #233      +/-   ##
==========================================
+ Coverage   95.10%   95.12%   +0.01%     
==========================================
  Files          18       18              
  Lines        1735     1742       +7     
==========================================
+ Hits         1650     1657       +7     
  Misses         85       85              

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

@eseiler
Copy link
Member Author

eseiler commented Feb 8, 2024

What does this do to the CWL export?

CTD looks ok, CWL has a duplicate base command?

Click to open
/path/to/app remote set-url --export-help {ctd,cwl}

<?xml version="1.0" encoding="UTF-8"?>
<tool ctdVersion="1.7" name="git-remote-set-url">
    <executableName><![CDATA[/path/to/app]]></executableName>
    <citations />
    <PARAMETERS version="1.7.0" xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/OpenMS/OpenMS/develop/share/OpenMS/SCHEMAS/Param_1_7_0.xsd" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
        <NODE name="remote" description="" tags="basecommand">
            <NODE name="set-url" description="" tags="basecommand">
                <ITEM name="positional_0" value="" type="string" description="" required="false" advanced="false" />
            </NODE>
        </NODE>
    </PARAMETERS>
</tool>

label: git-remote-set-url
doc: ""
inputs:
  []
outputs:
  []
cwlVersion: v1.2
class: CommandLineTool
baseCommand:
  - app
  - remote
  - set-url
  - set-url

@seqan-actions seqan-actions added lint [INTERNAL] signals that clang-format ran and removed lint [INTERNAL] signals that clang-format ran labels Feb 8, 2024
@seqan-actions seqan-actions added lint [INTERNAL] signals that clang-format ran and removed lint [INTERNAL] signals that clang-format ran labels Feb 8, 2024
@seqan-actions seqan-actions added lint [INTERNAL] signals that clang-format ran and removed lint [INTERNAL] signals that clang-format ran labels Feb 8, 2024
@seqan-actions seqan-actions added lint [INTERNAL] signals that clang-format ran and removed lint [INTERNAL] signals that clang-format ran labels Feb 8, 2024
mr-c
mr-c previously requested changes Feb 9, 2024
Copy link

@mr-c mr-c left a comment

Choose a reason for hiding this comment

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

I'd like to see a test to confirm sane CWL output

@seqan-actions seqan-actions added lint [INTERNAL] signals that clang-format ran and removed lint [INTERNAL] signals that clang-format ran labels Feb 9, 2024
@eseiler
Copy link
Member Author

eseiler commented Feb 9, 2024

I'd like to see a test to confirm sane CWL output

This'll need deNBI-cibi/tool_description_lib#50 #235

Might be a follow-up PR to add a test.
As far as I can tell, only the baseCommand isn't handled correctly.

@seqan-actions seqan-actions added lint [INTERNAL] signals that clang-format ran and removed lint [INTERNAL] signals that clang-format ran labels Sep 19, 2024
@seqan-actions
Copy link
Member

Documentation preview available at https://docs.seqan.de/preview/seqan/sharg-parser/233

@seqan-actions seqan-actions added lint [INTERNAL] signals that clang-format ran and removed lint [INTERNAL] signals that clang-format ran labels Sep 19, 2024
@eseiler eseiler dismissed mr-c’s stale review September 19, 2024 21:10

CWL test added

@eseiler eseiler marked this pull request as ready for review September 19, 2024 21:10
@seqan seqan deleted a comment from vercel bot Sep 19, 2024
@seqan-actions seqan-actions added lint [INTERNAL] signals that clang-format ran and removed lint [INTERNAL] signals that clang-format ran labels Sep 20, 2024
@SGSSGene SGSSGene self-requested a review September 23, 2024 11:36
Copy link
Contributor

@SGSSGene SGSSGene left a comment

Choose a reason for hiding this comment

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

Nice! Looks good to me!

@SGSSGene SGSSGene merged commit ef08477 into seqan:main Sep 23, 2024
33 checks passed
@eseiler eseiler deleted the misc/subcommands branch September 23, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants