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

Option to show the relative paths in the .pot comments #12438

Merged
merged 2 commits into from Aug 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/Shell/Task/ExtractTask.php
Expand Up @@ -49,6 +49,13 @@ class ExtractTask extends Shell
*/
protected $_merge = false;

/**
* Use relative paths in the pot files rather than full path
*
* @var bool
*/
protected $_relativePaths = false;

/**
* Current file being processed
*
Expand Down Expand Up @@ -233,6 +240,10 @@ public function main()
$this->_merge = strtolower($response) === 'y';
}

if (isset($this->params['relative-paths'])) {
Copy link
Member

Choose a reason for hiding this comment

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

You could use if (!$this->param('relative-paths')) { here.

$this->_relativePaths = !(strtolower($this->params['relative-paths']) === 'no');
}

if (empty($this->_files)) {
$this->_searchFiles();
}
Expand Down Expand Up @@ -321,6 +332,9 @@ public function getOptionParser()
])->addOption('merge', [
'help' => 'Merge all domain strings into the default.po file.',
'choices' => ['yes', 'no']
])->addOption('relative-paths', [
'help' => 'Use relative paths in the .pot file',
'choices' => ['yes', 'no']
Copy link
Member

Choose a reason for hiding this comment

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

This could be a boolean argument. That will default it to false and when --relative-paths is set on the CLI it will be true.

])->addOption('output', [
'help' => 'Full path to output directory.'
])->addOption('files', [
Expand Down Expand Up @@ -447,6 +461,9 @@ protected function _parse($functionName, $map)
'file' => $this->_file,
'line' => $line,
];
if ($this->_relativePaths) {
$details['file'] = '.' . str_replace(ROOT, '', $details['file']);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If anyone has a better idea for the string replacing here - please just change it!

Also using the . in front of the path is more of a personal experience. I couldn't see Cakephp using either ./ or / so I just picked my favorite.

Just tell me if I should change it.

Copy link
Member

Choose a reason for hiding this comment

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

I personally would just use foo/bar/baz.ctp without any leading dot or slash.

Copy link
Member

Choose a reason for hiding this comment

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

I usually replace ROOT . DS completely :)
But either way is already relative and works fine.
So this looks already good!

}
if (isset($plural)) {
$details['msgid_plural'] = $plural;
}
Expand Down
26 changes: 26 additions & 0 deletions tests/TestCase/Shell/Task/ExtractTaskTest.php
Expand Up @@ -184,6 +184,7 @@ public function testExtractWithExclude()
$pattern = '/\#: .*default\.ctp:\d+\n/';
$this->assertNotRegExp($pattern, $result);
}

/**
* testExtractWithoutLocations method
*
Expand Down Expand Up @@ -358,4 +359,29 @@ public function testExtractCore()
$pattern = '/#: Test\//';
$this->assertNotRegExp($pattern, $result);
}

/**
* test relative-paths option
*
* @return void
*/
public function testExtractWithRelativePaths()
{
$this->Task->interactive = false;

$this->Task->params['paths'] = TEST_APP . 'TestApp/Template';
$this->Task->params['output'] = $this->path . DS;
$this->Task->params['extract-core'] = 'no';
$this->Task->params['relative-paths'] = true;

$this->Task->method('in')
->will($this->returnValue('y'));

$this->Task->main();
$this->assertFileExists($this->path . DS . 'default.pot');
$result = file_get_contents($this->path . DS . 'default.pot');

$expected = '#: ./tests/test_app/TestApp/Template/Pages/extract.ctp:';
$this->assertContains($expected, $result);
}
}