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

:param v2 #962

Merged
merged 2 commits into from
Sep 6, 2019
Merged

:param v2 #962

merged 2 commits into from
Sep 6, 2019

Conversation

huboneo
Copy link
Contributor

@huboneo huboneo commented Aug 27, 2019

This PR adds support for the :param v2 syntax (explicit returns). In addition, it also cleans up and standardizes the error frame appearance regardless of error origin. Finally it standardizes the appearance of :help buttons.

Todo

  • Fix tests once @neo4j/browser-lambda-parser is published

STR

  • run :help param and follow the instructions for explicit returns

Screenshots

Setting parameters
Screenshot 2019-08-27 at 15 45 57

Error
Screenshot 2019-08-27 at 14 31 15

:help commands
Screenshot 2019-08-27 at 15 43 16

changelog: Support additional :param definition. See :help params in neo4j-browser for instructions.

@huboneo huboneo requested a review from oskarhane August 27, 2019 13:48
@huboneo huboneo force-pushed the feature-param-v2 branch 2 times, most recently from a999616 to 3d96f89 Compare August 27, 2019 13:54
}

// future proofing
if (parameters.type !== ARRAY) return null
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe return {}

Copy link
Member

@oskarhane oskarhane left a comment

Choose a reason for hiding this comment

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

I love the parser approach on this one. So great not to add more regexps!
Maybe we could think about adding another parser (or extend yours) to parse all :param and :params input? (not now, but might be worth a card).

It seems to have broken e2e tests.

package.json Outdated Show resolved Hide resolved
src/browser/modules/Help/html/param.html Show resolved Hide resolved
src/browser/modules/Stream/auto-exec-button.jsx Outdated Show resolved Hide resolved
- Added new dep @neo4j/browser-lambda-parser
- Updated existing cmd utils to use parser for all things lambda
- Standardized and cleaned up error frames appearance regardless of source
- Added new help text for :param
- Standardized appearance of :help buttons and Error frames
@huboneo
Copy link
Contributor Author

huboneo commented Sep 4, 2019

@oskarhane thank you for your feedback! I have addressed your comments. For the AutoExecButton tests I had to export a non-bus version as I could not figure out how to properly mock react-suber Please let me know if you have a better way

@huboneo huboneo requested a review from oskarhane September 4, 2019 14:10
Copy link
Member

@oskarhane oskarhane left a comment

Choose a reason for hiding this comment

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

LGTM

@huboneo huboneo merged commit 7eaf7c6 into neo4j:master Sep 6, 2019
@huboneo huboneo deleted the feature-param-v2 branch September 6, 2019 14:06
@huboneo huboneo mentioned this pull request Sep 11, 2019
3 tasks
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.

2 participants