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

debugger text improvment #2416

Merged
merged 5 commits into from
Nov 4, 2019
Merged

debugger text improvment #2416

merged 5 commits into from
Nov 4, 2019

Conversation

LianaHus
Copy link
Collaborator

@LianaHus LianaHus commented Oct 24, 2019

@@ -15,7 +15,7 @@ MemoryPanel.prototype.update = function (calldata) {
}

MemoryPanel.prototype.render = function () {
return yo`<div id='memorypanel' >${this.basicPanel.render()}</div>`
return yo`<div id='memorypanel' class='text-monospace small'>${this.basicPanel.render()}</div>`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use double quote for attribute value class="text-monospace small"

<div class="${css.stepDetail} column">${this.stepDetail.render()}</div>
</div>
</div>`
var headView = yo`
Copy link
Collaborator

Choose a reason for hiding this comment

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

use const

</div>
</div>`
var headView = yo`
<div id='vmheadView' class="${css.vmheadView} container">
Copy link
Collaborator

Choose a reason for hiding this comment

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

use double quote for attribute value id="..."

${this.fullStoragesChangesPanel.render()}
</div>
</div>`
var view = yo`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above const & doubleQuotes

@@ -11,7 +11,7 @@ StackPanel.prototype.update = function (calldata) {
}

StackPanel.prototype.render = function () {
return yo`<div id='stackpanel' >${this.basicPanel.render()}</div>`
return yo`<div id='stackpanel' class='text-monospace small' >${this.basicPanel.render()}</div>`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Double quotes for id and class

module.exports = {
formatSelf: formatSelf,
extractData: extractData
}

function formatSelf (key, data) {
var style = fontColor(data)
var keyStyle = data.isProperty ? 'color:#847979' : ''
var keyStyle = data.isProperty ? 'color: var(--info)' : ''
Copy link
Collaborator

Choose a reason for hiding this comment

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

use const

if (data.type === 'string') {
data.self = JSON.stringify(data.self)
}
return yo`<label style=${keyStyle}>${key}: <label style=${style}>${data.self}</label><label style='font-style:italic'> ${data.isProperty || !data.type ? '' : ' ' + data.type}</label></label>`
return yo `<label style='${keyStyle};white-space:pre-wrap;'> ${' ' + key}:<label style=${style}>${' ' + data.self}</label><label style='font-style:italic'> ${data.isProperty || !data.type ? '' : ' ' + data.type}</label></label>`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I cannot read this line. Use classes instead of setting the style directly, also format the code.
Also I don't understand why doing : ${' ' + key}, this shoud be ${key}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this should not be here.

@@ -55,16 +58,20 @@ function extractData (item, parent, key) {
}

function fontColor (data) {
var color = '#124B46'
var color = 'var(--primary)'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use let

data.type.indexOf('enum') === 0) {
color = '#0F0CE9'
color = 'var(--info)'
} else if (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Create a variable or a function for this if statement that explain what it's. Something like :

function isType(data, types) {
  return types.some(type => data.type[0] === type);
}
...
else if (isType(data, ['uint', 'int', 'bool', 'enum'])) { ... }

@@ -55,16 +58,20 @@ function extractData (item, parent, key) {
}

function fontColor (data) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add comment that describe what is the type. Here is an example from JSDoc:

/**
 * Assign the project to an employee.
 * @param {Object} employee - The employee who is responsible for the project.
 * @param {string} employee.name - The name of the employee.
 * @param {string} employee.department - The employee's department.
 */
Project.prototype.assign = function(employee) {
    // ...
};

@LianaHus LianaHus merged commit d63eb94 into master Nov 4, 2019
@LianaHus LianaHus deleted the debugger_text branch November 4, 2019 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants