1519315 - Display CodeMirror's gutter in console input when devtools.webconsole.input.editor is true

Status: RESOLVED FIXED

(DevTools :: Console, task, P1)

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1519315 - Display CodeMirror's gutter in console input when devtools.webconsole.input.editor is true; .

Depends on D17098

Comment 2

[George Ma]

Hi Nicolas, I'd love to take on this bug (thanks for the suggestion)! I'll get started on setting up the work environment today and read up on CodeMirror to get some background knowledge before diving into the code :)

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Hello George, I assigned the bug to you.
Don't hesitate to ask questions here or on Slack :)

Comment 4

[George Ma]

Hi Nicolas, first off sorry for the delay but had a chance to finally finish setting up my work environment tonight (had to use a VM), and began looking over the devtools code related to this bug. I have a couple of questions that I wanted to clarify with the task at hand if that's alright-

When you refer to console input, are you referring to this? And in that case, is my task to implement a way to display the CodeMirror gutters in a fashion similar to this video? (not mine)

Thanks in advance!
-George

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Hello George,

What we want to achieve here is the same thing than in the debugger or in the style editor, i.e. having the line number on the left.

In the devtools, we are using a shared component to wrap codeMirror, which we call the source editor (devtools/client/shared/sourceeditor/editor.js).

And by looking at the code there, you might get a sense of how it's working.
Also, this bug has already a patch attached that you can have a look at :) https://phabricator.services.mozilla.com/D17099

Comment 6

[George Ma]

Hi Nicolas, thanks for directing me to that shared component! I was able to find this line that helped me make sense of the code in the patch.

if (this.config.lineNumbers && !this.config.gutters.includes("CodeMirror-linenumbers")) {
	this.config.gutters.push("CodeMirror-linenumbers");
}

On my end, I got the console to show but only when setting in the Editor constructor, lineNumbers=true.

I tried the approach of adding an editorMode: PropTypes.bool field to propTypes() and setting lineNumbers=this.props instead, but realized that the reason this does not work is because I am not setting the field anywhere.

I will take a further look into where I can 1. check devtools.webconsole.input.editor=true, and then 2. set the lineNumbers field, tomorrow morning.

(any advice is welcome, especially if I’m off track completely)
Thanks! :)

-George

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Hello George, I think you're on the right track!
So, to get the editorMode prop in the JsTerm component, you need to pass it from the parent components.
I had a patch where I did this, https://phabricator.services.mozilla.com/D17096.
You can try to take some inspiration from there (only to retrieve and pipe down the editorMode prop).

Comment 8

[George Ma]

Hi Nicolas!

I’ve taken a look over that patch you sent(D17096), and I’ve tried to add some of the lines regarding editorMode similar to that of your App.js file.

In mapStateToProps(state) of JSTerm.js, I’ve added the line the following line:

// JSTerm.js

// Redux connect
function mapStateToProps(state){
    //...
    editorMode: state.ui.editor,
};

and looking further into what a redux connect statement is, (connects a React component to a Redux store), I added the corresponding line to the UiState of store.js, and to constants.js, similar to the code you added in that patch.

// store.js

ui: uiState({
    //...
	editor: getBoolPref(PREFS.UI.EDITOR),
}),

// constants.js

UI: {
    //...
    EDITOR: "devtools.webconsole.input.editor",
},

However, this approach did not display the lines in the gutter as needed, and I’m currently a little stumped (primarily due to my lack of react knowledge).

My current idea of what I could be missing is that I may need to add something to the render() function of JSTerm.js, as I’ve looked over the patch and noticed that in both theEditorToolbar.js and App.js file of the patch, you manipulate the editorMode variable (but do not explicitly set it).

Hopefully I conveyed my issue in a way that you can understand, but essentially I just need some guidance to what I’m missing regarding my attempt to pass down the editorMode prop from the parent component.

Thanks! :)

-George

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Thanks a lot George, looks like you're on the right track!
Could you push your commit to Phabricator so I can see what you might be missing? You can read https://docs.firefox-dev.tools/contributing/code-reviews-setup.html if you did not set it up yet (make sure to use moz-phab as it makes things easier).

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

Hello George, how are you doing?

It looks like Bug 1519313 will land soon, and it has all the piping to get the editorMode prop into the JSTerm, so that's a part you don't have to do :)

If you want to test it right away, before it's on mozilla-central, you can do

hg import https://hg.mozilla.org/integration/autoland/raw-rev/90de1c90ae69

Then you can rebase your work on top of that and see if the gutter is here when in editor mode :)

Comment 11

[George Ma]

Hi Nicolas! I checked out the bug and it looks like I was indeed on the right track, glad to know that my discussion with you most likely helped out Sola with the fixes! I believe the part I was missing on my end was the step where I had to run ./mach run with the necessary flags: ./mach run --devtools --setpref devtools.webconsole.input.editor=true. Running it with these flags and making a few small changes to get my codebase looking exactly like Sola's fix (only really had to add a few lines to App.js), makes the gutters show as intended now!

I rebased the code directly from Sola's bug as I couldn't get that hg import statement working, (the only line that needs to be added is the line found on this bug commit), and the gutters now seem to be working as intended. Let me know if this added line to his commit works for you, and thanks for all the help!

Comment 12

[Nicolas Chevobbe [:nchevobbe]]

Okay, this is perfect!
So now, what we want is a test to make sure we do show the gutter when in editor mode.
Again, you can check Sola's patch where he added a test as well.
You probably don't need to execute anything, but you will have to check if the gutter element is displayed. Let me know if something isn't clear enough!

Comment 13

[Nicolas Chevobbe [:nchevobbe]]

Hello George, how is it going on? Is anything blocking you?

Comment 14

[Nicolas Chevobbe [:nchevobbe]]

clearing assignee

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

Hello Bryan,

Would you be interested working on this bug? It's about the same feature you worked already :)

Comment 16

[Nicolas Chevobbe [:nchevobbe]]

Assigning to Bryan as discussed on Slack

Comment 17

[Bryan Kok [:transfusion]]

Attachment: Bug 1519315 - Display CodeMirror's gutter in console input when devtools.webconsole.input.editor is true. r=nchevobbe.

Comment 18

[Nicolas Chevobbe [:nchevobbe]]

Hello Bryan, how is this going?

Comment 19

[Bryan Kok [:transfusion]]

Hi, really sorry about falling off the radar. Thanks for pushing it over the finish line!

Comment 20

[Nicolas Chevobbe [:nchevobbe]]

sure! thanks for the patch you wrote! You'll still be credited as the author of the commit, so you can say you contributed to Firefox :)

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/938ba43a3350
Display CodeMirror's gutter in console input when devtools.webconsole.input.editor is true. r=Honza.

Comment 22

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/938ba43a3350