Add links to npm packages in package.json file view #29344

silverwind · 2024-02-23T10:36:47Z

When viewing package.json, this will add links to npmjs.com for all dependencies:

Two caveats:

The matching will have false-positives if any json object key matches one of the npm dependencies. I think it's very unlikely thought and even if there are false-positives, they don't hurt.
The code relies on chroma syntax class to detect the JSON key. Not ideal because I don't think chroma give stability guarantees for those, but I see no better way.

web_src/css/base.css

delvh

Hmm…
I'm simultaneously against and in favor of adding this special handling.
What it will most likely lead to is a barrage of other issues requesting special handling for other files as well.

However, I can also see why it is useful.
Is there perhaps a way to make this approach extensible, so that an admin can add scripts themselves to decide which files should receive special handling?
That way, we can then tell people wanting additional handling to do it themselves.

web_src/js/features/repo-code.js

silverwind · 2024-02-23T16:10:39Z

I'm simultaneously against and in favor of adding this special handling.
What it will most likely lead to is a barrage of other issues requesting special handling for other files as well.

Yeah it's kind of borderline. GitLab has this as well, FWIW, example.

Is there perhaps a way to make this approach extensible, so that an admin can add scripts themselves to decide which files should receive special handling?

Admins can already add custom scripts which would be able to do the same.

silverwind · 2024-02-23T16:21:13Z

I could certainly see this being extended to support more file types like go.mod, cargo.toml, pyproject.toml, requirements.txt, imports in go files, imports in js, etc. All easy to parse and alter. Kind of like what popular (but currently broken) github extension https://github.com/OctoLinker/OctoLinker did.

Extract from #29344. With this class it's possible to have links that don't color on hover. It will be useful for #29429.

Extract from go-gitea/gitea#29344. With this class it's possible to have links that don't color on hover. It will be useful for go-gitea/gitea#29429. (cherry picked from commit ffeaf2d0bd6c99c486aa7869779bb9ceb0aedad6)

* origin/main: (332 commits) Refactor external URL detection (go-gitea#29973) Refactor repo header/list (go-gitea#29969) Fix various loading states, remove `.loading` class (go-gitea#29920) Update register application URL for GitLab (go-gitea#29959) Refactor StringsToInt64s (go-gitea#29967) Switch to happy-dom for testing (go-gitea#29948) Performance improvements for pull request list page (go-gitea#29900) Refactor URL detection (go-gitea#29960) Solving the issue of UI disruption when the review is deleted without refreshing (go-gitea#29951) Fix JS error and improve error message styles (go-gitea#29963) Fix the bug that user may logout if he switch pages too fast (go-gitea#29962) Cancel previous runs of the same PR automatically (go-gitea#29961) Exclude `routers/private/tests` from air (go-gitea#29949) Remove codecov badge (go-gitea#29950) Misc color tweaks (go-gitea#29943) Fix and rewrite markup anchor processing (go-gitea#29931) Remove fomantic grid module (go-gitea#29894) Add background to dashboard navbar, fix missing padding (go-gitea#29940) Prevent layout shift in `<overflow-menu>` items (go-gitea#29831) Fix loadOneBranch panic (go-gitea#29938) ...

silverwind · 2024-03-21T23:24:24Z

Restructured into new file render/code.js and did a number of enhancements. Ready again.

silverwind · 2024-03-22T23:19:39Z

@wxiaoguang let me know if I bother you too much with those review request 😆

web_src/js/utils/misc.js

wxiaoguang · 2024-03-23T03:14:19Z

web_src/js/render/code.js

+  }
+
+  // match chroma NameTag token to detect JSON object keys
+  for (const el of document.querySelectorAll('.code-inner .nt')) {


It is really fragile, it doesn't seem suitable to be done on frontend.

It's the best frontend can do, and I don't think chroma would ever break it and even if it does, it will be graceful.

I think it's not right to let frontend do: the JS code only sees the rendered content, and it heavily depends on Chroma behavior.

I am sure there could be a stable backend solution: fully tested and fully controllable.

I'll not be doing that in backend and I think doing it in backend would massively limit future similar "postprocessing".

Doing these file processings in frontend enables to do more that what the backend could do with only HTML rendering. In this case it's possible to do in backend but on other more advanced cases like, frontend side will be required so I prefer to keep it in frontend only.

Take for example GitHub's code view. All the features there like "go to function" and symbol definitions are pure frontend features in React. The backend is just a dumb server of the content and I think that's very preferable.

I'll personally not be doing that in backend and I think doing it in backend would massively limit future similar processings.

I do not think it would limit any future processings. Instead, backend could handle all future processings better than frontend.

For example, if it needs to handle maven XML, in frontend, you need a lot of tricks to to handle the rendered XML-HTML content. But in backend, it sees the clear origin content and could add links clearly.

Have fun implementing a VSCode-like code view in backend 😆. I think these features definitely need to be in frontend and in fact can only be done there.

General talking about "frontend" and "backend" is not clear for this case. Let's abstract the logic:

origin content (text) -> highlight (get HTML) -> split to lines (HTML-line slice) -> output to page content

So the question is: add the links between which steps?

My "backend" proposal:

origin content (text) -> highlight AND/OR add links (get HTML) -> split to lines (HTML-line slice) -> output to page content

You "frontend" proposal:

origin content (text) -> highlight (get HTML) -> split to lines (HTML-line slice) -> output to page content -> use JS to parse the rendered HTML to add links

VSCode: VSCode has a well-designed plugin system, I haven't really looked into it, so I don't know at which step it could add links. My intuition tells me it is very unlikely to do it at the last step (eg: parse the rendered content). If I was a VSCode developer, I would do it like this:

origin content (text) -> parse (structured tree) -> plugin processing (add links) -> highlight (get HTML) -> output to editor

Adding only links or any other HTML content can be done in backend but my point is that if the post-processing goes beyong HTML modification, like for example a JSON "block folding" feature, this has do be done in JS and then it's better if all such post-processing code is in frontend (JS) and not split between backend and frontend.

Also, if we decide to render code using React or Vue, it will be much easier to migrate the JS code than to migrate it from Go.

lunny · 2024-04-09T08:25:49Z

Maybe we need a whole design resolution but not a case looks like hacky.

silverwind · 2024-04-09T09:35:29Z

I think doing it in frontend is reasonable. Backend would be better of course, but I'll not be willing to it in backend personally.

lunny · 2024-04-26T13:10:35Z

I mean maybe we should introduce a syntax analysis mechanism that supports LSP. So that not only this kind of file but all kinds of files can have references and jumps.

silverwind · 2024-04-27T10:51:12Z

Yes of course some intelligent view like GitHub has would be great long-term, but this likely requires persistent subprocesses for the LSP servers and integration against their protocol likely using websocket.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 23, 2024

pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 23, 2024

silverwind added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Feb 23, 2024

github-actions bot added the modifies/frontend label Feb 23, 2024

silverwind force-pushed the npmlink branch 4 times, most recently from c01247c to 888e9af Compare February 23, 2024 10:42

Add links to npm in package.json file view

d617918

silverwind force-pushed the npmlink branch from 888e9af to d617918 Compare February 23, 2024 10:43

silverwind added 4 commits February 23, 2024 12:10

factor out createExternalLink

405e737

tweak test

fdb81a7

rename

f57ecc7

extract another function

1a12f18

silverwind changed the title ~~Add links to npm in package.json file view~~ Add links to npm packages in package.json file view Feb 23, 2024

silverwind commented Feb 23, 2024

View reviewed changes

web_src/css/base.css Outdated Show resolved Hide resolved

delvh reviewed Feb 23, 2024

View reviewed changes

web_src/js/features/repo-code.js Outdated Show resolved Hide resolved

web_src/js/features/repo-code.js Outdated Show resolved Hide resolved

silverwind added 2 commits February 23, 2024 17:46

add new suppressed link helper and use it

c472295

add comment

d7b4a34

silverwind marked this pull request as draft February 23, 2024 23:14

silverwind mentioned this pull request Mar 16, 2024

add .suppressed link class #29847

Merged

silverwind added a commit that referenced this pull request Mar 16, 2024

add .suppressed link class (#29847)

ffeaf2d

Extract from #29344. With this class it's possible to have links that don't color on hover. It will be useful for #29429.

github-actions bot added the modifies/js label Mar 21, 2024

silverwind added 3 commits March 22, 2024 00:13

restructure and improve

33b66c7

revert change

9b2f650

add more keys

8aa5c5e

silverwind marked this pull request as ready for review March 21, 2024 23:23

silverwind added 2 commits March 22, 2024 00:27

fix lint

c2217a4

fix lint

1f3a151

silverwind requested a review from wxiaoguang March 22, 2024 23:19

wxiaoguang reviewed Mar 23, 2024

View reviewed changes

web_src/js/utils/misc.js Outdated Show resolved Hide resolved

wxiaoguang reviewed Mar 23, 2024

View reviewed changes

silverwind added 2 commits March 23, 2024 12:39

remove unneeded attributes and add comments

c28fe8b

change createExternalLink to createLink

53ff681

pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 23, 2024

silverwind added 2 commits March 23, 2024 12:46

format

1dd1343

add check

3dedb85

silverwind removed the modifies/frontend label Mar 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add links to npm packages in package.json file view #29344

Add links to npm packages in package.json file view #29344

silverwind commented Feb 23, 2024 •

edited

Loading

delvh left a comment

silverwind commented Feb 23, 2024 •

edited

Loading

silverwind commented Feb 23, 2024 •

edited

Loading

silverwind commented Mar 21, 2024

silverwind commented Mar 22, 2024

wxiaoguang Mar 23, 2024

silverwind Mar 23, 2024 •

edited

Loading

wxiaoguang Mar 23, 2024

silverwind Mar 23, 2024 •

edited

Loading

wxiaoguang Mar 23, 2024 •

edited

Loading

silverwind Mar 23, 2024 •

edited

Loading

wxiaoguang Mar 23, 2024 •

edited

Loading

silverwind Mar 23, 2024 •

edited

Loading

silverwind Mar 23, 2024

lunny commented Apr 9, 2024

silverwind commented Apr 9, 2024 •

edited

Loading

lunny commented Apr 26, 2024

silverwind commented Apr 27, 2024 •

edited

Loading

Add links to npm packages in package.json file view #29344

Are you sure you want to change the base?

Add links to npm packages in package.json file view #29344

Conversation

silverwind commented Feb 23, 2024 • edited Loading

delvh left a comment

Choose a reason for hiding this comment

silverwind commented Feb 23, 2024 • edited Loading

silverwind commented Feb 23, 2024 • edited Loading

silverwind commented Mar 21, 2024

silverwind commented Mar 22, 2024

wxiaoguang Mar 23, 2024

Choose a reason for hiding this comment

silverwind Mar 23, 2024 • edited Loading

Choose a reason for hiding this comment

wxiaoguang Mar 23, 2024

Choose a reason for hiding this comment

silverwind Mar 23, 2024 • edited Loading

Choose a reason for hiding this comment

wxiaoguang Mar 23, 2024 • edited Loading

Choose a reason for hiding this comment

silverwind Mar 23, 2024 • edited Loading

Choose a reason for hiding this comment

wxiaoguang Mar 23, 2024 • edited Loading

Choose a reason for hiding this comment

silverwind Mar 23, 2024 • edited Loading

Choose a reason for hiding this comment

silverwind Mar 23, 2024

Choose a reason for hiding this comment

lunny commented Apr 9, 2024

silverwind commented Apr 9, 2024 • edited Loading

lunny commented Apr 26, 2024

silverwind commented Apr 27, 2024 • edited Loading

silverwind commented Feb 23, 2024 •

edited

Loading

silverwind commented Feb 23, 2024 •

edited

Loading

silverwind commented Feb 23, 2024 •

edited

Loading

silverwind Mar 23, 2024 •

edited

Loading

silverwind Mar 23, 2024 •

edited

Loading

wxiaoguang Mar 23, 2024 •

edited

Loading

silverwind Mar 23, 2024 •

edited

Loading

wxiaoguang Mar 23, 2024 •

edited

Loading

silverwind Mar 23, 2024 •

edited

Loading

silverwind commented Apr 9, 2024 •

edited

Loading

silverwind commented Apr 27, 2024 •

edited

Loading