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

Binary foreign key enhancement #13894 #14202

Merged
merged 1 commit into from May 3, 2018

Conversation

@williamdes
Copy link
Member

@williamdes williamdes commented Apr 15, 2018

Patch for binary foreign key insert/edit dropdown bug
Fixes: #13894

@williamdes williamdes force-pushed the williamdes:issue-13894-bin-fk-bug branch 3 times, most recently from 34c0087 to 49bd7af Apr 15, 2018
@codecov
Copy link

@codecov codecov bot commented Apr 15, 2018

Codecov Report

Merging #14202 into master will decrease coverage by <.01%.
The diff coverage is 18.75%.

@@             Coverage Diff              @@
##             master   #14202      +/-   ##
============================================
- Coverage     50.31%    50.3%   -0.01%     
- Complexity    14367    14373       +6     
============================================
  Files           502      502              
  Lines         66997    67013      +16     
============================================
+ Hits          33708    33711       +3     
- Misses        33289    33302      +13
@williamdes williamdes force-pushed the williamdes:issue-13894-bin-fk-bug branch 4 times, most recently from e6742aa to 33154b1 Apr 19, 2018
@@ -1199,7 +1200,7 @@ public function purgeHistory($username)
*
* @access protected
*/
public function buildForeignDropdown(array $foreign, $data, $mode)
public function buildForeignDropdown($column, array $foreign, $data, $mode)

This comment has been minimized.

@mauriciofauth

mauriciofauth Apr 25, 2018
Member

Hi, @williamdes. Thanks for your contribution.

I don't know if it's necessary to create this $column parameter.

Is it possible to do something like this?

if (($_SESSION['tmpval']['display_binary']
&& $meta->type === self::STRING_FIELD)
|| ($_SESSION['tmpval']['display_blob']
&& stristr($meta->type, self::BLOB_FIELD))
) {
// in this case, restart from the original $content
if (mb_check_encoding($content, 'utf-8')
&& !preg_match('/[\x00-\x08\x0B\x0C\x0E-\x1F\x80-\x9F]/u', $content)
) {
// show as text if it's valid utf-8
$result = htmlspecialchars($content);
} else {
$result = '0x' . bin2hex($content);
}
list(
$is_truncated,
$result,
// skip 3rd param
) = $this->_getPartialText($result);
}

@williamdes williamdes force-pushed the williamdes:issue-13894-bin-fk-bug branch from 33154b1 to 7335e58 Apr 26, 2018
Closes: #13894
Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes williamdes force-pushed the williamdes:issue-13894-bin-fk-bug branch from 7335e58 to e9d3fdd May 2, 2018
@williamdes williamdes changed the title Patch for binary foreign key insert/edit dropdown bug #13894 Binary foreign key enhancement #13894 May 2, 2018
@mauriciofauth mauriciofauth merged commit 3bf396c into phpmyadmin:master May 3, 2018
3 of 6 checks passed
3 of 6 checks passed
Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
codecov/patch 18.75% of diff hit (target 50.31%)
Details
codecov/project 50.3% (-0.01%) compared to 35e13ea
Details
DCO All commits have a DCO sign-off from the author
Details
Scrutinizer Analysis: 2 updated code elements – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mauriciofauth
Copy link
Member

@mauriciofauth mauriciofauth commented May 3, 2018

Merged, thanks for your contribution!

@mauriciofauth mauriciofauth self-assigned this May 3, 2018
@mauriciofauth mauriciofauth added this to the 5.0.0 milestone May 3, 2018
@williamdes williamdes deleted the williamdes:issue-13894-bin-fk-bug branch May 3, 2018
@williamdes williamdes restored the williamdes:issue-13894-bin-fk-bug branch May 9, 2018
@williamdes
Copy link
Member Author

@williamdes williamdes commented May 9, 2018

[...]
Do I create (another another issue & PR) or (a commit to this PR) for adding the ability to search through hex data ?

@mauriciofauth
Copy link
Member

@mauriciofauth mauriciofauth commented May 9, 2018

@williamdes You can open a new issue, as it's a new issue, even though they are related.

@williamdes
Copy link
Member Author

@williamdes williamdes commented May 9, 2018

@mauriciofauth I will do that.
I added a commit to this branch to fix my error.

@williamdes williamdes mentioned this pull request May 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants