Skip to content

Conversation

@ota-meshi
Copy link
Member

@ota-meshi ota-meshi commented Apr 3, 2024

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[x] Other, please explain: This PR adds a code path explorer page.

What changes did you make? (Give an overview)

This PR adds a code path explorer page.
Also, since this PR is the first page implementation, it also includes source code such as the build system.

Is there anything you'd like reviewers to focus on?

There is currently the following remaining task.

  • Does not support syntax highlighting for DOT syntax. (I don't know yet how to implement them with codemirror.)
  • Wrap mode is not supported. (I don't know yet how to implement them with codemirror.)
  • I haven't set up GitHub Actions yet.

I would also like to hear your opinion on the following:

  • This page is currently built using Vite. If we prefer webpack we will need to change some configs. Do we have a preferred build system?
  • Will this source eventually be moved to eslint.org like in the playground? In that case, we will need to change to share the stylesheet as well.

@eslint-github-bot

This comment was marked as resolved.

@ota-meshi ota-meshi changed the title Add code path explorer feat: add code path explorer Apr 3, 2024
@ota-meshi ota-meshi marked this pull request as ready for review April 5, 2024 11:16
@ota-meshi
Copy link
Member Author

Although it doesn't support some options yet, I think you've pretty much got the page for viewing code paths.
The remaining work on the code path page that I am aware of is as follows.

  • Does not support syntax highlighting for DOT syntax.
  • Wrap mode is not supported.
    (I don't know yet how to implement them with codemirror.)
image image

@ota-meshi
Copy link
Member Author

A few things to note:

  • This page is currently built using Vite. If we prefer webpack we will need to change some configs.
  • I haven't set up GitHub Actions yet.
  • Will this source eventually be moved to eslint.org like in the playground? In that case, we will need to change to share the stylesheet as well.

@fasttime
Copy link
Member

fasttime commented May 1, 2024

What is the status of this PR? I'm asking because there is no description.

@ota-meshi
Copy link
Member Author

Thank you for letting me know. I forgot to write a description at the top of this PR 😅. Now I wrote it.
There are a few remaining tasks that I don't seem to be able to solve any time soon. If we're willing to put them on the back burner, I think we're ready for you to review them.

@fasttime
Copy link
Member

Thanks for the update @ota-meshi. I think we can definitely start to review this, and maybe someone will know how to fix the remaining tasks. The repo is still private, so only team members will be able to see it anyway.

@fasttime fasttime added the accepted There is consensus among the team that this change meets the criteria for inclusion label May 10, 2024
@nzakas
Copy link
Member

nzakas commented May 13, 2024

There is currently the following remaining task.

Does not support syntax highlighting for DOT syntax. (I don't know yet how to implement them with codemirror.)

That's definitely okay for the first draft. We can look at doing this later.

Wrap mode is not supported. (I don't know yet how to implement them with codemirror.)

It looks like there's an extension for that:
https://codemirror.net/docs/ref/#view.EditorView%5ElineWrapping

I would also like to hear your opinion on the following:

This page is currently built using Vite. If we prefer webpack we will need to change some configs. Do we have a preferred build system?

We don't have a preferred build system. If you did it using Vite, then let's stick with that.

Will this source eventually be moved to eslint.org like in the playground? In that case, we will need to change to share the stylesheet as well.

No. We'll keep this as a separate site at code-explorer.eslint.org.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

Thanks for getting this started, and sorry I missed it.

Some overall comments:

  1. Can you please add our standard file headers to each js/jsx file? That would really help to understand how things are organized.
  2. Similarly, can you add some comments in the js/jsx files explaining what is happening? Especially for someone like me who hasn't used React, there's a lot going on that's difficult to follow.
  3. Can you add some documentation explaining how the app is structured overall?

"node": ">=18"
},
"scripts": {
"build:website": "vite build",
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a start script that runs vite?

@nzakas
Copy link
Member

nzakas commented May 13, 2024

Looks like we have some alignment issues. (Firefox on Windows)
Screenshot 2024-05-13 at 10-53-08 ESLint Explorer

@ota-meshi
Copy link
Member Author

ota-meshi commented Jun 12, 2024

I think I've addressed some of the issues. But some challenges remain.
If you have time, could you please exclude the following issues and comment if you have any additional issues?

  • It looks like there's an extension for that:
    https://codemirror.net/docs/ref/#view.EditorView%5ElineWrapping
  • Similarly, can you add some comments in the js/jsx files explaining what is happening? Especially for someone like me who hasn't used React, there's a lot going on that's difficult to follow.
  • Can you add some documentation explaining how the app is structured overall?

The remaining issues will be addressed when I have time.

@nzakas
Copy link
Member

nzakas commented Jun 12, 2024

Thanks @ota-meshi. Because we'd like to get this done soon, would you mind if we brought on someone else to finish this?

@ota-meshi
Copy link
Member Author

would you mind if we brought on someone else to finish this?

Of course it's okay 👍

@nzakas
Copy link
Member

nzakas commented Jul 11, 2024

Just FYI: we do have someone working on this, they just haven't pushed changes yet. Hopefully soon.

@ota-meshi ota-meshi closed this Jul 23, 2024
nzakas pushed a commit that referenced this pull request Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion feature

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants