fix eslint and prettier errors #216
Conversation
| @@ -125,7 +127,7 @@ const mapDispatchToProps = dispatch => ({ | |||
| if (file) { | |||
| if (file.includes('/lib/') || file.includes('/data/') || file.includes('/audio/')) { | |||
| console.log('files under /lib/, /data/, and /audio/ cannot be run as a script'); | |||
| return undefined; | |||
| return; | |||
There was a problem hiding this comment.
Eslint is expecting consistent return values, so we either need to explicitly return undefined in all code paths, or return void (which should end up being undefined anyway).
| const project_version = props.project.get('version'); | ||
| const project_url = props.project.get('project_url'); | ||
| const projectVersion = props.project.get('version'); | ||
| const projectUrl = props.project.get('project_url'); |
There was a problem hiding this comment.
Renamed snake_case variables to camelCase
| @@ -44,6 +44,8 @@ | |||
| margin-right: 10px; | |||
| border: 1px solid transparent; | |||
| color: var(--interactive-medium-default); | |||
| background: none; | |||
| font-family: monospace; | |||
There was a problem hiding this comment.
Changed this element from a div to a button to address lint errors related to accessibility. That affected its appearance so I added these styles which I think brings it back in line with the original appearance.
| className={editorOptionClasses(key, val)} | ||
| onClick={() => this.updateConfiguration(key, val)} | ||
| > | ||
| {label} | ||
| </div> | ||
| </button> |
There was a problem hiding this comment.
Change to a button to assist users using keyboards or screen readers.
| @@ -51,12 +51,12 @@ class Editor extends Component { | |||
|
|
|||
| keyService.bindings.forEach(({ keystroke }) => { | |||
| const cpIndex = keymap.findIndex(x => x.keys === keystroke.vimKey); | |||
| if (~cpIndex) { | |||
| if (cpIndex !== -1) { | |||
There was a problem hiding this comment.
I'm not 100% certain on the intent here, but it seems like this has the same behavior as ~cpIndex. ~cpIndex will be truthy unless cpIndex is -1.
| @@ -222,6 +222,23 @@ export const bufferRead = resource => dispatch => { | |||
| }); | |||
| }; | |||
|
|
|||
| export const directoryRead = resource => dispatch => { | |||
There was a problem hiding this comment.
Moved this definition up to fix a use-before-definition error.
| } | ||
| } | ||
|
|
||
| export const commandService = new CommandService(); |
There was a problem hiding this comment.
Moved this definition up to fix a use-before-definition error.
| successArr = successArr.sort(orderResultsByProjectName).filter(response => response.successResult.updated); | ||
| success = [...successArr] | ||
| .sort(orderResultsByProjectName) | ||
| .filter(response => response.successResult.updated); |
There was a problem hiding this comment.
Fix an error caused by reassigning a function argument. I also created a shallow copy of the original argument because sort mutates the array in place.
| }; | ||
|
|
||
| export const orderResultsByProjectName = (a, b) => a.name < b.name ? -1 : a.name > b.name ? 1 : 0; | ||
| export const orderResultsByProjectName = (a, b) => a.name.localeCompare(b.name); |
There was a problem hiding this comment.
Fix an error due to nested ternary-if statements. I replaced the logic with String.prototype.localeCompare which has the same behavior.
|
Thanks for the PR! Fixing the lint and formatting stuff is long overdue. I’m trying to remember why we backed away from it in the early days. I’ll try to review today or tomorrow in more detail! |
|
so so sorry for the delay in reviewing and merging. time has been in short supply and i prioritized a bunch of kernel work to support the 2022 shield hardware. |
I'm planning on tackling some of the "good first issue" issues in this repo. However, I noticed that I'm getting a lot of lint errors in VSCode, so I decided to run
yarn lint --fixfirst. I did have to manually fix a few spots which I will call out with comments.Thanks for considering these changes!