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

Refactor scheme architecture #3348

Merged
merged 19 commits into from
Mar 4, 2024
Merged

Refactor scheme architecture #3348

merged 19 commits into from
Mar 4, 2024

Conversation

aadcg
Copy link
Member

@aadcg aadcg commented Feb 26, 2024

Description

A pre-requisite to #2989, in the sense that it refactors the architecture of custom schemes in a renderer-agnostic fashion. It is a follow-up to #3341.

Checklist:

  • Git branch state is mergable.
  • Changelog is up to date (via a separate commit).
  • New dependencies are accounted for.
  • Documentation is up to date.
  • Compilation and tests ((asdf:test-system :nyxt/gi-gtk))
    • No new compilation warnings.
    • Tests are sufficient.

@jmercouris jmercouris self-requested a review February 26, 2024 16:04
@aadcg aadcg force-pushed the refactor-scheme-architecture branch from 538a5d6 to 51e6315 Compare February 26, 2024 17:55
@aadcg aadcg marked this pull request as ready for review February 26, 2024 18:03
@jmercouris
Copy link
Member

I have been looking at this, and haven't found any problem yet. I will continue to review it.


(export-always 'internal-page-name)
(-> internal-page-name ((or string quri:uri)) t)
(defun internal-page-name (url)
Copy link
Member

Choose a reason for hiding this comment

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

Since this is often used as a predicate, perhaps consider a -p suffix.

Copy link
Member

Choose a reason for hiding this comment

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

Or at least a variant of this command that is -p suffixed that returns t/nil.

@jmercouris
Copy link
Member

jmercouris commented Mar 1, 2024

The manual command works, but not when clicked from the button on the home page.

It even works if I set the url to nyxt:manual from the home page.

home page == nyxt:new

@jmercouris
Copy link
Member

If I click on a symbol within a code block, it also does not work. I am brought again to the debugger.

@aadcg
Copy link
Member Author

aadcg commented Mar 1, 2024

The manual command works, but not when clicked from the button on the home page.

If I click on a symbol within a code block, it also does not work. I am brought again to the debugger.

I've seen these bugs before, I'll fix those.

@aadcg aadcg force-pushed the refactor-scheme-architecture branch from 51e6315 to 9c6dad0 Compare March 4, 2024 18:07
@aadcg
Copy link
Member Author

aadcg commented Mar 4, 2024

@jmercouris should be fixed, please test it.

@jmercouris
Copy link
Member

Will do!

@jmercouris
Copy link
Member

No problems found :-)

@aadcg aadcg force-pushed the refactor-scheme-architecture branch from 9c6dad0 to a056537 Compare March 4, 2024 21:02
@aadcg aadcg merged commit 13e2a7d into master Mar 4, 2024
@aadcg aadcg deleted the refactor-scheme-architecture branch March 4, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants