renderer/gtk(context-source): Parse the existing contexts.

[aartaka]

Description

This fixes the problem described in #3124: existing user-defined contexts are only listed if there are buffers with them. So the inactive contexts (the ones having a directory but not used in any buffer) are not listed. This parses the data directory to find context directories and lists them.

Fixes #3124

Discussion

Should we sort contexts for display? Feels like an unnecessary optimization.

Checklist:

Everything in this checklist is required for each PR. Please do not approve a PR that does not have all of these items.

• Git hygiene:
  • I have pulled from master before submitting this PR
  • There are no merge conflicts.
• I've added the new dependencies as:
  • ASDF dependencies,
  • Git submodules,

    cd /path/to/nyxt/checkout
    git submodule add https://gitlab.common-lisp.net/nyxt/py-configparser _build/py-configparser

  • and Guix dependencies.
• My code follows the style guidelines for Common Lisp code. See:
  • Norvig & Pitman's Tutorial on Good Lisp Programming Style (PDF)
  • Google Common Lisp Style Guide
• I have performed a self-review of my own code.
• My code has been reviewed by at least one peer. (The peer review to approve a PR counts. The reviewer must download and test the code.)
• Documentation:
  • All my code has docstrings and :documentations written in the aforementioned style. (It's OK to skip the docstring for really trivial parts.)
  • I have updated the existing documentation to match my changes.
  • I have commented my code in hard-to-understand areas.
  • I have updated the changelog.lisp with my changes if it's anything user-facing (new features, important bug fix, compatibility breakage).
    • Changelog update should be a separate commit.
  • I have added a migration.lisp entry for all compatibility-breaking changes.
  • (If this changes something about the features showcased on Nyxt website) I have these changes described in the new/existing article at Nyxt website or will notify one of maintainters to do so.
• Compilation and tests:
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works. (If possible.)
  • I ran the tests locally ((asdf:test-system :nyxt) and (asdf:test-system :nyxt/gi-gtk)) and they pass.

[jmercouris]

Thank you for this fix! Can you provide me a recipe to test this? I'm not very familiar with the usage of contexts in Nyxt.

[aartaka]

Thank you for this fix! Can you provide me a recipe to test this?

Sure.

• Open Nyxt.
• Invoke make-buffer-with-context.
• Input the name for a new context, e.g. foo.
• Use switch-buffer to check that the new buffer indeed has a foo context.
• Invoke make-buffer-with-context to see that foo context is available.
• Close the buffer with foo.
• Invoke make-buffer-with-context again. That's where the behavior differs noticeably:
  • Before this patch, foo disappears and you cannot choose it from the context prompt.
    • Well, you actually can, if you input it and create a "new" context from raw input.
  • With this patch, foo is listed as part of existing sources and you can choose it.

[jmercouris]

I see. I personally feel the function of listing the contexts available should be extracted to a function and then invoked by the constructor rather than being fully embedded within the form. What do you think?

[aartaka]

I see. I personally feel the function of listing the contexts available should be extracted to a function and then invoked by the constructor rather than being fully embedded within the form. What do you think?

Yes. Done in c574c2f. Merging?

[aartaka]

Refactored list-existing-contexts in a cleaner way, rebased, and squashed. Ready to merge.

[aadcg]

@jmercouris please validate. Thanks.

[jmercouris]

Looks much better, I will give it a test and then approve!

[jmercouris]

That was a mistake...

[jmercouris]

Unfortunately it didn't work on my system. I made a context "programming", navigated, then killed the buffer, and it did not appear in the suggestions.

I did check ~/.local/share/nyxt and the folder programming-web-context exists there. Does it work on your system still? How are your contexts stored?

My folder looks like this:

[-] nyxt
 |--[+] default-web-context
 |--[+] deviceidhashsalts
 |--[+] extensions
 |--[+] history
 |--[-] programming-web-context
 |   |--[+] deviceidhashsalts
 |   |--[+] localstorage
 |   `--[+] storage
 |--[+] storage
 |--[+] style-mode-css-cache
 |----- annotations.lisp
 |----- auto-rules.lisp
 |----- bookmarks.lisp
 |----- bookmarks.lisp.bak
 |----- cookies.txt
 |----- default-cookies
 |----- feeds.lisp
 |----- nyxt.log
 `----- programming-cookies

[jmercouris]

list-existing-contexts returns Nil for me.

[jmercouris]

Rewriting list-existing-contexts as the following fixes it on my system:

(defun list-existing-contexts ()
  (loop for dir in (uiop:subdirectories
                    (files:expand (make-instance 'nyxt:nyxt-data-directory)))
        ;; PATHNAME-DIRECTORY returns a list on most implementations (UIOP
        ;; relies on that, at least), even though the standard doesn't mandate
        ;; it.
        for dirname = (alex:lastcar (uiop:ensure-list (pathname-directory dir)))
        do (print dirname)
        when (uiop:string-suffix-p dirname "web-context")
          collect (subseq dirname
                          0
                          (- (length dirname) (length "-web-context")))))

was it working on yours with the trailing slash? If so, how?

[aartaka]

Right, that's a silly mistake left over from the previous implementation. Fixed now!

[jmercouris]

I'll try it again soon. I must re-emphasize, all pull requests should be functional. One should test them before submitting.

[Ambrevar]

Style: Prefer sera:drop (or sera:slice) to subseq.

[Ambrevar]

Better and more reliable: files:basename.

[aartaka]

@Ambrevar, thanks for suggestions! Cleanup pushed as c9857de to master.