-
Notifications
You must be signed in to change notification settings - Fork 283
Fixed Generator::mkdir when already exists #525
Conversation
👍 Status: reviewed. @HeahDude thanks for fixing this ... and I'm sorry for having introduced it :( |
@@ -72,8 +72,9 @@ protected function renderFile($template, $target, $parameters) | |||
*/ | |||
public static function mkdir($dir, $mode = 0777, $recursive = true) | |||
{ | |||
mkdir($dir, $mode, $recursive); | |||
self::writeln(sprintf(' <fg=green>created</> %s', self::relativizePath($dir))); | |||
if (@mkdir($dir, $mode, $recursive)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about checking for existence instead of silencing the operation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 for @xabbuh proposal
@@ -72,7 +72,8 @@ protected function renderFile($template, $target, $parameters) | |||
*/ | |||
public static function mkdir($dir, $mode = 0777, $recursive = true) | |||
{ | |||
if (@mkdir($dir, $mode, $recursive)) { | |||
if (!is_dir(dirname($dir))) { | |||
mkdir($dir, $mode, $recursive); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep the silencing operator (Symfony always does @mkdir
) because eventually an error will happen here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests are failing anyway, let's revert this one.
2f24c9a
to
c43c993
Compare
@@ -72,8 +70,10 @@ protected function renderFile($template, $target, $parameters) | |||
*/ | |||
public static function mkdir($dir, $mode = 0777, $recursive = true) | |||
{ | |||
mkdir($dir, $mode, $recursive); | |||
self::writeln(sprintf(' <fg=green>created</> %s', self::relativizePath($dir))); | |||
if (!is_dir(dirname($dir))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be $dir
, not dirname($dir)
, no? same on the next line as well
if (!is_dir(dirname($target))) { | ||
self::mkdir(dirname($target)); | ||
} | ||
self::mkdir($target); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be dirname($target)
Thanks @fabpot we're good to go now :) |
Thank you @HeahDude. |
This PR was merged into the 3.1.x-dev branch. Discussion ---------- Fixed Generator::mkdir when already exists Closes #523. Commits ------- 18b58df Fixed Generator::mkdir when already exists
Closes #523.