Open igoralmeida opened 3 years ago
Thank you for your exploration :)
This issue has a few questions for several different problems, and most of them are probably because, besides having to learn nyxt internals, I'm also trying to learn Common Lisp as I go :) Please excuse the rambling, and let me know if I should post this in your discourse instead of here.
As long as it's focused (and I believe it is) and has to do with how can we improve Nyxt for extensibility -- Issues are fine!
Right now, however, it is not very ergonomic. So, for the 'building UIs' part of this discussion, a first question would be: what is the proper way to do this, if I want to navigate up and down that list (say, with
k
/j
), and have other keys operate on the currently-highlighted thread, for examplea
for archive,+
to edit the tags, etc? This would be similar to how it's done in astroid, which I'm definitely trying to emulate.
Right now, what you need to do is to write some Parenscript/Javascript and call it inside Nyxt commands. So, to edit the tags, you need to access these in JS-land, return these as JSON string, parse it on Lisp side and prompt the user with the list of these. You can look at hints-add
and query-hints
in element-hint.lisp for inspiration and patterns to use. I can try to write the example command if you need it, but that'll take some time. That's quirky, but that's the best way to do this as of now.
Second question: how should I organize this? I am not sure I understand how classes and slots can help me here. My current thought is to create a class for the search command's output buffer, so that slots would be parameters like the current search string; similarly, there would be a class for the show command's output buffer. Even if that is the way I should do it, I did not find a way to "modify" a slot and then "reload" the buffer so that the new parameters would take effect. I'm probably missing or misunderstanding concepts here.
That feels like a re-implementation of our in-progress UI library :) What you hint at is the lack of basic actionable UI element (call it line, box, whatever; every anchor should be this element instead. So, to make it easy to create such lists, we need thes elements to:
current-element
to the currently focused one and make it usable in commands, like
(define-command edit-tags (&optional (element (ui:current-element)))
(setf (tags (data element))
(prompt-minibuffer :input-prompt "Choose tags"
:suggestion-function (tag-suggestion-filter (tags (data element))))))
@jmercouris, how does it look? It seems general-purpose and useful enough to be in our UI lib!
Third question: how should I distribute this as a separate package, since it should definitely NOT be in nyxt core? Currently it is a single script at the root of the nyxt repo, that I load manually from sly. My thinking here was that my
~/.config/nyxt/init.lisp
could somehow point to this new package, and nyxt would load it during initialization; but I'm not sure this is possible since I need thedefine-mode
s anddefine-command
s, etc.
You should distribute it as a separate ASDF system in a separate repo. Guidelines are:
nx-
prefix for discoverability.(:use #:nyxt)
or (:import-from #:nyxt ... #:define-command ...)
(latter is preferable) to invoke Nyxt-specific functions/macros in your code without nyxt:
prefix.And that's it. If your package is distributed as a well-formed ASDF system, it will be easy to load to Nyxt via load-after-system
.
One example of extension package is https://github.com/kssytsrk/nx-freestance-handler. You can draw some inspiration from it :)
I have definitely not read everything I should have to be able to tackle this, so pointers to reading material are also very welcome.
There's little material on how to extend Nyxt and how to write UIs in it. First I am currently working on, second... I'm not sure :upside_down_face: I guess we'll write about it when we make UI library more or less complete.
Finally, if there IS infrastructure in place to do this, great, but it's fine if there isn't. Hopefully this issue will serve to highlight the possibilities.
Yes, thanks for bringing that up! UI library needs improvement and making it work for your notmuch interface can be a good test for it :)
EDIT: &optional.
Kudos for this, the screenshot looks amazing! :)
Our "extension infrastructure" is still a bit immature considering we have only 1 extension :p So if you run into any issue, please report, we can work on it together!
Thank you @aartaka for the pointers and @Ambrevar for the support :) I will work on a very simple proof-of-concept of the other main 'tasks' so we can discuss some more.
Hi,
Here's a peek of the thread view:
Again, zero interactivity other than selecting the anchor in the search view. However, I am struggling with new lines in the body of messages:
The problem seems to be that the reader doesn't like that notmuch uses \n
in the strings and I wonder how I could get around this without actually resorting to (the probably correct thing) fetching raw message bodies separately.
Here's an experiment, for clarity. A simple test script prints a plist, like notmuch would do:
#!/bin/bash
echo "(:id \"Hello\nWorld\")"
... that run-program
reads directly, like I'm doing:
CL-USER> (uiop:run-program '("./test.sh") :output :form)
(:ID "HellonWorld")
NIL
0 (0 bits, #x0, #o0, #b0)
After some searching and monkeying around I thought I could get cl-interpol to parse it for me:
CL-USER> (uiop:run-program '("./test.sh") :output #'(lambda (s) (cl-interpol:interpol-reader s nil nil :recursive-p nil)))
":id \"Hello
World\""
NIL
0 (0 bits, #x0, #o0, #b0)
but I understand even less of that package as I do of nyxt, so I'm a bit lost here. Obviously I can manually wrap the fake plist in test.sh
in another parentheses and call read-from-string
inside the lambda, but that won't be possible for notmuch.
It doesn't REALLY matter since this is experimental, but it would be nice to not have to worry about a proper external interface for now since the point is to see if nyxt can work for the UI.
Happy to hear any thoughts you might have.
Wow, this is very exciting stuff! :)
About the \n
: indeed, \n is just "n" for the Common Lisp reader.
So Interpol can do the job for you as you figured out.
You could strip the \n manually, it's easy:
(str:replace-all "\n" (string #\newline) "(:id \"Hello\nWorld\")")
But maybe there is another library that works like cl-interpol without stripping the parentheses.
@jmercouris @aartaka Any idea?
There is no difference between an escaped n and a regular n, though. And the escape is already lost once the string is read:
CL-USER> (str:replace-all "\n" (string #\newline) "(:id \"Testing Hello\nWorld\")")
"(:id \"Testi
g Hello
World\")"
T
Edit: in fact, the first argument to replace-all is already NOT an indicator of a newline character, it's simply an n.
The only thing I can think of is using a <pre>
tag. I'm sorry, no other ideas!
How about piping shell command output to a file and reading the file afterwards on Lisp side? Do I make sense? :)
I don't think this file intermediary is necessary. It's a bit more cumbersome + it's inefficient, I'm sure we can do this directly by using a custom reader, like cl-interpol does.
cl-interpol does seem to work, but I could not find a reliable way to read-from-string
afterwards.
In my previous comment, I showed this output:
CL-USER> (uiop:run-program '("./test.sh") :output #'(lambda (s) (cl-interpol:interpol-reader s nil nil :recursive-p nil)))
":id \"Hello
World\""
NIL
0 (0 bits, #x0, #o0, #b0)
The behavior here seems to be that the opening paren is being treated as the outer delimiter for the parser.
Eventually I added START-OF-TEXT and END-OF-TEXT with (push '(#\Stx . #\Etx) cl-interpol:*outer-delimiters*)
, which (apparently) solved the string problem.
(uiop:run-program '("./test.sh")
:output #'(lambda (s)
(read-from-string
(cl-interpol:interpol-reader
(make-concatenated-stream ; TODO dirty hack
(make-string-input-stream "")
s
(make-string-input-stream ""))
nil nil :recursive-p nil)))))
(And I just realized that, by sending this message, I'll be creating an e-mail which has the exact unicode characters I'm assuming never show up :) )
This allows test.sh to have very happy emoticons like :)))))))
and never break anything, but as soon as I use it with the notmuch command-line, some e-mail will have the exact combination of colons or close-parens that eventually read-from-string
cannot properly decode, and I get weird errors like
Package HTTP does not exist.
Stream: #<dynamic-extent STRING-INPUT-STREAM (unavailable) from "((((:id ...">
[Condition of type SB-INT:SIMPLE-READER-PACKAGE-ERROR]
At this point I don't see any other way forward other than fetching the message body separately.
On a separate topic, how can I implement "refresh" with these "internal buffers" (?) I'm creating? My search command is basically a call to (with-current-html-buffer (buffer buffer-name 'nyxtmuch-mode) ...)
, and I tried to create a keymap to override the r
key, but I wasn't sure what to do after this
(defvar nyxtmuch--search-keymap (make-keymap "nyxtmuch-search-map"))
(define-key nyxtmuch--search-keymap
"r" 'nyxtmuch--search-reload)
(define-command nyxtmuch--search-reload ()
;???
)
(define-mode nyxtmuch-mode ()
"mode for nyxtmuch"
((keymap-scheme (keymap:make-scheme
scheme:vi-normal nyxtmuch--search-keymap))))
Waaaaait no need to resort to dirty hack if the goal is to just load a text buffer and convert all the \n
into newlines :)
The problem is with how the CL reader handles \n
in strings. But if you don't deal with strings, then you are fine.
So what you can do is deal with byte arrays instead, and from there replace the 92 110
bytes with 10
.
This should work.
I'll ask around if there is a more immediate way with some library or with a custom reader.
About the reload command: you don't really need the nyxtmuch--search-keymap
nor the define-key
. Look at how os-package-manager-mode
does it (or web-mode
).
Then in the command you can basically run the code that refreshes the buffer. This command should probably be the one that holds the (with-current-html-buffer ...)
code.
Look at how message-mode
does it.
I found this for handling C escape chars: https://github.com/williamyaoh/trivial-escapes.
As it turns out, the sexp format was not very robust and I got a few annoying errors with double quotes in headers (even without the message body). Eventually I switched to the json interface and yason, and by that point I had already caved and was fetching the message body separately with cl-mime, so the whole newline escaping thing seems moot.
Following up on my 'second question' in my starting comment, I feel like I'm doing too much work with my own class for search buffers. The problem is that, to make a 'refresh' keybinding work, I would need to know a buffer's search string:
(define-class nyxtmuch-search-buffer (user-internal-buffer)
((search-string "" :type string :documentation "Notmuch search string for this buffer.")
(default-modes '(nyxtmuch-search-mode vi-normal-mode web-mode base-mode))
(style ...))
;exports, etc
)
(define-command nyxtmuch-render-search (&optional buffer)
"Build the nyxtmuch search associated with this buffer"
(let* ((buffer (or buffer (current-buffer)))
(style (style buffer))
(search-string (search-string buffer)))
(with-current-buffer buffer
(html-set
(markup:markup ...)))))
Then the search command would basically show a prompt, instantiate one of those and call the search rendering command.
So with this choice, I realized I could not use the new buffer that with-current-html-buffer
gave me (wrong class), so the search command essentially had to repeat the buffer-creating logic found there:
(define-command nyxtmuch-search ()
"Open nyxtmuch with some search"
(let* ((search-term (prompt-minibuffer ...))
(buffer-name ...)
(mybuf (make-instance
'nyxtmuch-search-buffer
:id (get-unique-buffer-identifier *browser*)
:title buffer-name
:search-string search-term)))
;repeating...
(hooks:run-hook (buffer-before-make-hook *browser*) mybuf)
(initialize-modes mybuf)
(buffers-set (id mybuf) mybuf)
(hooks:run-hook (buffer-make-hook *browser*) mybuf)
;build it
(nyxtmuch-render-search mybuf)
(set-current-buffer mybuf)))
Should buffer-make
take a type argument to avoid repeating myself here? Or is this design wrong?
Besides simply adding the search-string slot, since I wanted to continue using internal-buffer's style sheet, the style
slot in my class is something like
;...
(style #.(str:concat
(slot-value (make-instance 'internal-buffer) 'style)
(cl-css:css
'(...
Since I may have to do a few more of these specific buffer classes with some of this 'merging slots' and instantiating behavior, it feels like I may be missing an abstraction here.
That said, right now refreshing does work, which is at least encouraging :)
It seems I was too clever with that style bit at the end, it does work to reeval the define-class form while everything is running, but not from a fresh sly session :)
Thank you for your thorough report. We've had a similar discussion with regards to diff-mode
. Perhaps buffer-make
should be expanded as you suggest. I'm not sure. I don't think you are missing anything here with regards to the abstraction.
The problem is the following: we cannot make an instance of a mode and then pass it to a buffer. We can only tell the buffer itself to instantiate an instance of a mode. On one hand, this makes some sense. What is a mode without a buffer? Is it possible for a mode to exist outside the context of a buffer? On the other hand, it adds challenges like the ones you are experiencing.
@Ambrevar do you have any suggestions?
As it turns out, the sexp format was not very robust and I got a few annoying errors with double quotes in headers (even without the message body).
S-exp should be as robust as JSON, if not more. Quoting is well handled when prin1-to-string and the like.
Maybe we can investigate this together.
S-exp is also usually faster than JSON.
About the mode / argument question, I agree we may lack a parameter in buffer-make
.
Can you expand on why with-current-html-buffer
does not cut it for you?
Probably because some arguments need to be passed to the mode during its creation
As it turns out, the sexp format was not very robust and I got a few annoying errors with double quotes in headers (even without the message body).
S-exp should be as robust as JSON, if not more. Quoting is well handled when prin1-to-string and the like.
I think my problem was this:
(uiop:run-program ... :output #'(lambda (s)
(read-from-string (cl-interpol:interpol-reader s nil nil :recursive-p nil)))
(which is like this because I was asking notmuch to include message bodies).
If I understand what happened, when confronted with something like ... (:headers (:subject "..." :from "\"Some name\" <mail@address.com>" :to ...) ...)
, cl-interpol strips the backslashes (I'm assuming), so in the resulting (malformed) plist, :from
would have an empty string followed by elements SOME
, NAME
and " <mail@address.com>"
(this I can see in sly), which eventually breaks a getf
call.
In this particular instance, I can simply change to :output :form
(which was basically what I had before the problem with newlines :) ).
We can revisit this eventually, but when performance becomes an issue we should probably work with libnotmuch instead of shell outputs anyway.
Can you expand on why with-current-html-buffer does not cut it for you?
Basically, I could not find a way to 'attach' information (like the notmuch search string) to the user-internal-buffer (?) that with-current-html-buffer
was giving me, so I had to create my own class. But then I have to instantiate it (and introduce it to the browser) myself, otherwise with-current-html-buffer
cannot find it; so I switched to with-current-buffer
+ html-set
as you can see above.
So my question was whether buffer-make
could take a type symbol so I don't have to do that ;repeating...
part in my code. I would still have to copy some of the initial code of the with-current-html-buffer
macro, but I think it's minimal.
Also, I hope I don't sound like I am teaching you people how nyxt works :) I'm simply stating my own understanding of how it fits together so you can point me in the right direction if I'm off.
These pasted snippets are starting to get too large, which can be a little frustrating to read without enough context. I'll try to publish this as an asdf system as aartaka suggested, so I can link to stuff instead.
No worries, your complaint is valid! with-current-html-buffer
is a back-of-the-envelop helper at best!
@jmercouris: Can you merge the buffer-make parameter change you made in #1242 ? This should fix @igoralmeida issue if I understand correctly.
Oh boy, now I do have to figure out how to split said commit :-D
Let me know if it does not work.
I know the mechanics of it, I meant the logical separation :-O
Done: 1f05dc64 !
Perfect!
Getting it to work with the new prompter library before I can move 'nyxtmuch' to a separate package, but this looks unintended, or at the very least confusing:
(Not even using my own code to call prompter, this is just the effect of pressing f
and c
to select one of the hints).
Sorry, what do you find confusing? That the "Hint" column is too wide?
Sorry, what do you find confusing? That the "Hint" column is too wide?
"D" is the first suggestion, although "C" is inputed. This is the issue Igor means, I believe.
Ha indeed, I ran into this issue myself recently, but couldn't reproduce.
@igoralmeida Can you produce a recipe? I think I know how to fix it, but I'd need a recipe so that we can test it.
Hmm... Assuming "recipe" is just a generic name for "steps to reproduce this":
(define-command testing-hint ()
"Do the thing"
(let ((buffer-name "*Testing*"))
(with-current-html-buffer (buffer buffer-name 'base-mode)
(markup:markup
(:style (style buffer))
(:p "Here's the link I want to select:")
(:p (:a :href "https://google.com" "Quick brown fox"))
(:p "And here's the link that gets highlighted as soon as I press the hint for the first link:")
(:p (:a :href "https://example.com" "lazy dog"))))))
testing-hint
f
to get hintsC
and the next one is D
, but you're not sure where you want to go yetc
and Enter without noticing that D
at the bottom was selectedNotice that if you change the second link to "example.net", pressing c
does nothing, which makes me think the ".com" part is what is being matched... (but then I wonder why not stay at the first, which has "quick"...).
There's probably a very lengthy and philosophical discussion on whether hints and search text should be mixed, maybe best to have it in a separate issue.
Hope that helps
Excellent, this is very helpful. I'll fix it very soon.
@igoralmeida do you have a repository where we can see the development of this extension? That would be very interesting to me!
I was in the process of moving it, following aartaka's instructions, but there seems to be a weird name collision and I'm still trying to understand what is happening. I'll publish what I have so far later today and we can discuss some more.
https://github.com/igoralmeida/nx-notmuch
Clicking a search result works, but following a hint does not.
I get Warning: Error on separate thead: The variable [ is unbound.
Are you using CCL?
Nope, sbcl
On April 11, 2021 6:51:23 AM GMT-03:00, Pierre Neidhardt @.***> wrote:
Are you using CCL?
-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/atlas-engineer/nyxt/issues/1190#issuecomment-817280200 -- Igor Almeida (móvel)
I've fixed the hinting issue with f05de064cfa0b502a57ff308d7bee8438188cc25.
And I've fixed the follow-hint
issue in https://github.com/atlas-engineer/nyxt/pull/1325.
Thanks for your patience and thanks for reporting all these issues!
Thanks! I was actually writing up my attempts at fixing things yesterday, but had to stop before finishing the post :) I'll take a look at your issues in my repo as well and report back.
Here's a peek of a slightly useful interaction (focus, collapse message body):
Note that read messages are collapsed by default, again emulating astroid.
Since there's no need to change the notmuch side, this is a good first test for how the UI could work.
Took me a while to find the correct javascript incantations, but in general "collapse" is hiding the message body div, and C-j
and C-k
are simple commands that call this script:
(define-parenscript focus-change (direction)
(defun clip (idx len)
(max (min idx (1- len)) 0))
(defun prev-element (current-idx collection)
(elt collection (clip (1- current-idx) (length collection))))
(defun next-element (current-idx collection)
(elt collection (clip (1+ current-idx) (length collection))))
(defun update-stuff ()
(ps:let* ((messages (nyxt/ps:qsa document "li.message"))
(old-element (nyxt/ps:qs document "li.message.selected"))
(new-element nil)
old-pos)
(when old-element
(setf (ps:@ old-element class-name) "message")
(setf old-pos (ps:chain *array prototype index-of (call messages old-element)))
(setf new-element (ps:lisp (case direction
(prev '(prev-element old-pos messages))
(next '(next-element old-pos messages))))))
(unless new-element
(setf new-element (elt messages 0)))
(setf (ps:@ new-element class-name) "message selected")
nil))
(update-stuff)
;;TODO scroll if necessary
)
To avoid reinventing any wheels, though, is this something you guys are actively working on right now? The focus is basically relying on classes for the li
tags, which could be fragile, if at least because there will be HTML from message bodies as well.
Keeping focus between refreshes is also something I'm not entirely sure how to achieve, since nyxtmuch-render-thread
erases everything and starts over. One idea would be to replicate the list and compare some kind of identity, but this seems wasteful.
I'll upload the code for this after some cleaning.
To avoid reinventing any wheels, though, is this something you guys are actively working on right now?
Are you asking if we are working on a UI library for Nyxt content? We do have a starting point in libraries/user-interface
but it's currently a bit primitive and it does not support lists / tree if I'm not mistaken.
In https://github.com/atlas-engineer/nyxt/pull/1593 we are working on displaying a buffer tree, which is pretty similar to what you are doing.
For the class, what about you add a nyxtmuch-whatever
class to your li
elements and match against that?
You could also add a nyxtmuch-body
class to the email body to avoid matching against it.
Following up here: I used CFFI to talk to libnotmuch and avoid buffering entire searches before rendering, and to make this work I basically added some parenscript that takes a search result and adds it to the list. In a nutshell, the iterator gives me things and I render them into the buffer.
I've been avoiding keeping a copy of the information on the CL side, but there'll probably be situations where this might be necessary. So I think this could be a possible addition to user-interface: some sort of information source yields items that the UI adds and maintains on the buffer, and commands that alter any items trigger an update of their 'view'. I did not grasp enough of the code in download-mode.lisp to understand if this is already possible.
Overall, the search is looking pretty usable and only a little dangerous (huge searches will go on until they're over or something/someone gives up :) ), which brings me to my next question: should the rendering code above be inside nyxt:run-thread
, so I could interrupt it if necessary?
Overall, the search is looking pretty usable and only a little dangerous (huge searches will go on until they're over or something/someone gives up :) ), which brings me to my next question: should the rendering code above be inside
nyxt:run-thread
, so I could interrupt it if necessary?
Yes, that should work. Not sure how you'd interrupt it, though.
My first guess would be to keep a reference to the thread, so that a "stop" command could use it. Wouldn't that work?
My first guess would be to keep a reference to the thread, so that a "stop" command could use it. Wouldn't that work?
Yes, that'd work :)
Hi everyone. First of all: nyxt is awesome and I love the concept and the possibilities it will bring, so thanks for all your work so far.
This issue has a few questions for several different problems, and most of them are probably because, besides having to learn nyxt internals, I'm also trying to learn Common Lisp as I go :) Please excuse the rambling, and let me know if I should post this in your discourse instead of here.
Basically, I was kind of inspired by #887 and started monkeying around to get nyxt to show my notmuch html mails. I'm not there yet, but at least I managed to show something on the screen. Here's what it looks like:
This is a very basic "search" command that essentially calls
uiop:run-program
and transforms notmuch's sexp output into markup. Each line is an anchor withlisp-url
, so I can use link hints (there would be 5 link hints in this case) and call a "show" command that creates a buffer with that particular e-mail thread (not yet implemented). I can also call that search command again to open another buffer, this time specifying different search terms.Right now, however, it is not very ergonomic. So, for the 'building UIs' part of this discussion, a first question would be: what is the proper way to do this, if I want to navigate up and down that list (say, with
k
/j
), and have other keys operate on the currently-highlighted thread, for examplea
for archive,+
to edit the tags, etc? This would be similar to how it's done in astroid, which I'm definitely trying to emulate.Second question: how should I organize this? I am not sure I understand how classes and slots can help me here. My current thought is to create a class for the search command's output buffer, so that slots would be parameters like the current search string; similarly, there would be a class for the show command's output buffer. Even if that is the way I should do it, I did not find a way to "modify" a slot and then "reload" the buffer so that the new parameters would take effect. I'm probably missing or misunderstanding concepts here. Also, searches can return thousands of threads, so the communication between nyxt and notmuch should be something else instead of the shell (astroid uses libnotmuch). But that can come later.
Third question: how should I distribute this as a separate package, since it should definitely NOT be in nyxt core? Currently it is a single script at the root of the nyxt repo, that I load manually from sly. My thinking here was that my
~/.config/nyxt/init.lisp
could somehow point to this new package, and nyxt would load it during initialization; but I'm not sure this is possible since I need thedefine-mode
s anddefine-command
s, etc.I have definitely not read everything I should have to be able to tackle this, so pointers to reading material are also very welcome. Finally, if there IS infrastructure in place to do this, great, but it's fine if there isn't. Hopefully this issue will serve to highlight the possibilities.
Thanks again