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

Add DICTIONARY #12

Merged
merged 2 commits into from Mar 5, 2019

Conversation

Projects
None yet
2 participants
@phoe
Copy link
Collaborator

phoe commented Feb 22, 2019

No description provided.

(format T "<p>Antonyms: ~{~{~% <a href=#~A>~A</a>~}~^,~}~%</p>~%"
(mapcar #'anchorize antonyms))))))))

(define-widget browser (qtextbrowser)

This comment has been minimized.

@Shinmera

Shinmera Feb 23, 2019

Owner

This name is too general, please change it to something that conveys this browser is specific to dictionaries.

This comment has been minimized.

@phoe

phoe Mar 5, 2019

Author Collaborator

Fixed.


(defmethod set-focus-from-browser ((dictionary dictionary) old new))

(defun %set-focus-from-browser (dictionary new)

This comment has been minimized.

@Shinmera

Shinmera Feb 23, 2019

Owner

What's the point of this extra function? Why not just use the method?

This comment has been minimized.

@phoe

phoe Mar 5, 2019

Author Collaborator

Fixed.

:initarg :not-found-text))
(:default-initargs
:empty-browser-text
"<i><p align=center>Type your query below and hit Search.</p></i>"

This comment has been minimized.

@Shinmera

Shinmera Feb 23, 2019

Owner

This is not valid HTML. p is a block tag, and i is an inline tag, so the nesting should be switched.

This comment has been minimized.

@phoe

phoe Mar 5, 2019

Author Collaborator

Fixed.

:not-found-text
"<i><p align=center>The entry for \"~A\" was not found.</p><i>"))

(define-subwidget (dictionary layout) (q+:make-qgridlayout)

This comment has been minimized.

@Shinmera

Shinmera Feb 23, 2019

Owner

If you (q+:make-whateverlayout my-widget) you don't need to (setf (q+:layout ..) ..) after. It's cleaner and shorter.

This comment has been minimized.

@phoe

phoe Mar 5, 2019

Author Collaborator

Fixed - TIL. I was not aware of this.

This comment has been minimized.

@Shinmera

Shinmera Mar 5, 2019

Owner

You don't need to comment on each thing as fixed. If you push the patches it'll notice that it's changed automatically.

This comment has been minimized.

@phoe

phoe Mar 5, 2019

Author Collaborator

Woooo. I keep on learning GitHub stuff every day.


(define-subwidget (dictionary browser)
(make-instance 'browser :dictionary dictionary)
(q+:add-widget layout browser 0 0 1 2)

This comment has been minimized.

@Shinmera

Shinmera Feb 23, 2019

Owner

I much prefer defining the layout last and adding the widgets in the layout's initialiser, as the positioning is more layout-specific information.

This comment has been minimized.

@phoe

phoe Mar 5, 2019

Author Collaborator

Fixed.

@Shinmera Shinmera merged commit 8e82b92 into master Mar 5, 2019

@phoe phoe deleted the dictionary branch Mar 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.