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

Added URL instance property for CLIRequest class #1082

Closed
wants to merge 1 commit into from

Conversation

hollax
Copy link

@hollax hollax commented Jun 24, 2018

In the Services.php file, the URL instance is passed to CLIRequest constructor but it not in use in the CLIRequest class. Consequently calling $this->request->url in the controller throws an exception when you access the app from command line $ php index.php [controller]. This is in the Services.php file under system config

/**
	 * The CLI Request class provides for ways to interact with
	 * a command line request.
	 *
	 * @param \Config\App $config
	 * @param bool        $getShared
	 *
	 * @return \CodeIgniter\HTTP\CLIRequest
	 */
	public static function clirequest(\Config\App $config = null, $getShared = true)
	{
		if ($getShared)
		{
			return self::getSharedInstance('clirequest', $config);
		}

		if (! is_object($config))
		{
			$config = new \Config\App();
		}

		return new \CodeIgniter\HTTP\CLIRequest(
			$config, new \CodeIgniter\HTTP\URI()
		);
	}

@hollax
Copy link
Author

hollax commented Jun 24, 2018

I also discovered that $uri->getSegments() returns empty array. I assume because the request was from command line. I fixed this in the CLIRequest constructor but it is not in this pull request. Since it was written that one change per request in the guidelines.

/**
	 * Constructor
	 *
	 * @param App $config
         * @param URI $uri
	 */
	public function __construct(App $config, $uri = null)
	{
		parent::__construct($config);

		// Don't terminate the script when the cli's tty goes away
		ignore_user_abort(true);
                
                $this->uri = $uri;
		$this->parseCommand();
                
                //set url path based command on parsed argument, this sets the segments
                $this->uri->setURI($this->getPath());
	}

@lonnieezell
Copy link
Member

While that might fix an error - it won't get you correct information from the URI instance. CLI requests don't have the same concepts as an HTTP request that has gone through a server and has additional information that the URL can be built through. That's why there's not support for the URI object.

I'll remove the URI instance being passed in so there's no confusion. It appears to be an artifact left over from earlier days.

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.

2 participants