Skip to content

Storm help command to show library and type information (SYN-6001) #3335

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

Merged
merged 29 commits into from
Sep 14, 2023

Conversation

vEpiphyte
Copy link
Contributor

No description provided.

@vEpiphyte vEpiphyte added enhancement reqChangelog requires changelog labels Sep 8, 2023
@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Patch coverage: 93.47% and project coverage change: -0.13% ⚠️

Comparison is base (715de85) 97.28% compared to head (99677b9) 97.16%.
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3335      +/-   ##
==========================================
- Coverage   97.28%   97.16%   -0.13%     
==========================================
  Files         228      228              
  Lines       46014    46323     +309     
==========================================
+ Hits        44767    45008     +241     
- Misses       1247     1315      +68     
Flag Coverage Δ
linux 97.16% <93.47%> (-0.03%) ⬇️
linux_replay ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
synapse/lib/stormhttp.py 100.00% <ø> (ø)
synapse/lib/autodoc.py 95.50% <90.06%> (-4.05%) ⬇️
synapse/lib/storm.py 95.71% <96.17%> (-0.01%) ⬇️
synapse/cortex.py 96.99% <100.00%> (-0.50%) ⬇️
synapse/lib/stormtypes.py 98.75% <100.00%> (+<0.01%) ⬆️

... and 8 files with indirect coverage changes

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

assert len(rtype) > 1
tdata = ', '.join(rtype)
lines.append('Returns:')
lines.append(f' The type may be one of the following: {tdata}.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is schema valid but we don't have anything which uses it implemented, so it is not covered in tests.

isgtor = False
isctor = False

if rname == 'ctor' or 'ctor' in rname:
Copy link
Contributor

Choose a reason for hiding this comment

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

You could probably just do the XXX in rname check for these since it will catch the rname == XXX case also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So these values are not schema bound, I'm not certain if its correct to allow a substring match - something like setting the type value to ctorxxxx would appear correct here but its allowing a typo to do the right thing?

Comment on lines 3056 to 3059
if item is not None and \
not isinstance(item, str) and \
not isinstance(item, s_stormtypes.Lib) and \
not callable(item):
Copy link
Contributor

Choose a reason for hiding this comment

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

You're missing the None case in this sequence of ifs.

Suggested change
if item is not None and \
not isinstance(item, str) and \
not isinstance(item, s_stormtypes.Lib) and \
not callable(item):
if item is None or \
(not isinstance(item, str) and \
not isinstance(item, s_stormtypes.Lib) and \
not callable(item)):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

self.opts.item being None indicates either 1) The user did not provide input ( the valid help case to list commands ) or 2) The user provided an input value which resolved to None ( help $lib.null ).

vEpiphyte and others added 4 commits September 11, 2023 18:03
Co-authored-by: blackout <blackout@vertex.link>
…nt resolution. support and directly as well.

register telepath proxy method types and give them docstrings.
self.stormIsInPrint('Get the value of the primary property of the Node.', msgs)
msgs = await alist(core.storm('[test:str=uniq] | help $node.value()'))

msgs = await core.stormlist('[test:str=uniq] | help $node.value()')
self.stormNotInPrint('get the value of the primary property of the Node.', msgs)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you didn't change this but I notice a similar line with a capital G a few lines above and I'm wondering if this assertion is passing because of a mismatch in case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test shows the difference between the help value being a bound method $node.value and the resolved value $node.value() which is the string uniq which is then matched against the list of commands and types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the typo though!

MichaelSquires
MichaelSquires previously approved these changes Sep 14, 2023
Cisphyx
Cisphyx previously approved these changes Sep 14, 2023
@vEpiphyte vEpiphyte merged commit 0f755cd into master Sep 14, 2023
@vEpiphyte vEpiphyte deleted the feat_help_libv2 branch September 14, 2023 17:12
@vEpiphyte vEpiphyte added this to the v2.149.0 milestone Sep 14, 2023
@vEpiphyte vEpiphyte removed the reqChangelog requires changelog label Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants