-
Notifications
You must be signed in to change notification settings - Fork 28
Add code and UAST buttons in results table #52
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
Conversation
@@ -13,8 +15,15 @@ class ResultsTable extends Component { | |||
switch (typeof v) { | |||
case 'boolean': | |||
return v.toString(); | |||
case 'string': | |||
// assume any multiline string is code | |||
if (v.split('\n').length > 1) { |
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.
Would this in JS work for both platform-dependent line endings?
Would it be possible to include a simple test for this function, so further refactoring would rely on it?
On the side-note, in case >= 1 could work for this, may be some kind of equivalent of .indeOf > 0
could be more performant..
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.
windows line ending is \r\n
so it will work.
👍 for indexOf
.
Also, here and in other pray rebase would make review life a bit easier. |
changed split to indexOf. Added simple test. Though I don't see any value in it. Current logic is completely wrong (for example commit messages can be displayed as code according to it). And I don't think it's possible to solve on frontend. But we agreed to go with this simple-wrong way for now. |
@@ -14,8 +16,15 @@ class ResultsTable extends Component { | |||
switch (typeof v) { | |||
case 'boolean': | |||
return v.toString(); | |||
case 'string': | |||
// assume any multiline string is code | |||
if (v.indexOf('\n') > 1) { |
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 this be > -1
? The way it is a multiline file with a blank line will not be detected.
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.
👍
Signed-off-by: Maxim Sukharev <[email protected]>
Signed-off-by: Maxim Sukharev <[email protected]>
Signed-off-by: Maxim Sukharev <[email protected]>
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.
👍
Based on #38
Fix: #23