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

Impossible to disable initial command #780

Closed
freethejazz opened this issue Jun 5, 2018 · 2 comments · Fixed by #850
Closed

Impossible to disable initial command #780

freethejazz opened this issue Jun 5, 2018 · 2 comments · Fixed by #850

Comments

@freethejazz
Copy link

freethejazz commented Jun 5, 2018

Description

It used to be possible to disable the initial command by passing in an empty initial command. The new version of the browser (currently testing on browser version 3.1.7) does not appear to allow this behavior.

Reproduce Steps

Set the initCmd value to a blank value via any of the following ways:

In the browser:

:config initCmd: ''

in neo4j.conf:

browser.post_connect_cmd=''

as an environment variable when starting up neo4j:

NEO4J_browser_post__connect__cmd=

Expected Behavior

The browser accepts the configuration parameter and respects it by not running the default command of :play intro

Current Behavior

The browser accepts the configuration parameter but does not respect it. If the configuration parameter is a falsey value, such as an empty string, the default command is run.

Can you propose a solution?

The beginning of this issue can be solved by adjusting the following line of code:

export const getInitCmd = state => state[NAME].initCmd || initialState.initCmd

If the initCmd is defined but falsey, the initialState.initCmd value is used instead (:play intro)

Simply altering that line to read something like:

export const getInitCmd = state => state[NAME].initCmd !== undefined ? state[NAME].initCmd : initialState.initCmd

may work, but there may be downstream issues when attempting to execute a command that's just an empty string.

I'm up for creating a PR for this if you all find it valuable. Thanks and keep up the great work!

@oskarhane
Copy link
Member

Good catch @freethejazz!
This is definitely a classic js bug, checking a falsey value on something when empty string (or zero in many cases) are something you want to handle.

And you're right, an empty string will probably be interpreted as cypher and sent to the server and return with an error message.
So either we need to validate the initCmd command and not pass it forward or we need to set a guard before sending cypher to the server.
Personally I think validating the initCmd and stop it early if empty is preferred.

If you'd like to submit a PR for it, that'd be awesome!

@oskarhane
Copy link
Member

Ping @freethejazz

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 a pull request may close this issue.

2 participants