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

icon.png config in 2 places #11

Open
wilbowma opened this issue Sep 22, 2014 · 3 comments
Open

icon.png config in 2 places #11

wilbowma opened this issue Sep 22, 2014 · 3 comments

Comments

@wilbowma
Copy link
Contributor

I noticed when trying to debug another issue that the path to icon.png is configurable in info.rkt, but is also hardcoded in client-gui.rkt (line 709).

wilbowma added a commit to nuprl/cs2500-client that referenced this issue Sep 22, 2014
wilbowma added a commit to nuprl/cs2500-grades that referenced this issue Sep 22, 2014
@elibarzilay
Copy link
Member

There is a very loose connection between the two. The entry in info.rkt is there for DrRacket, and it is part of the protocol of defining tools. (IIRC, it's only used to show an icon on the DrR splash screen.) There could be a few icons there for several tools; you might want to avoid the hadin icon there completely; and they can appear in any order. (And just this makes the fix that you pushed to your repo two weeks ago bogus.) The other uses are for the toolbar icon and an icon on the standalone multi-submission tool.

So the current state is that there is no real option to specify your own file -- it should always be icon.png, which is what the comment in the template info file and the docs say:

Replace icon.png in your renamed directory [...]

(and even the icon in the repo indicates that it should be replaced...) So the thing is that most of the handin client content should be used as is, except for the template stuff that should be copied and modified, and that includes info.rkt, icon.png and server-cert.pem. (In my setup, my actual collection has symlinks to the handin repo for everything, except for these three files.)


The correct way to do this would be to add another info field, and use that later in the tool specs:

(define icon-file "icon.png")
(define drracket-tools      `("client-gui.rkt"))
(define drracket-tool-names `(,name))
(define drracket-tool-icons `(,icon-file))

and then use it also in the other source files. But this seems like an overkill which wasn't needed so far. I think that the most we got to in the past was people who installed two handin clients, and it was confusing when both were the NEU logo, but even in that case there was no issue with using the icon.png name. Given how extensively it was (maybe still is) used here, I'd be surprised if there's anywhere else that needed that abstraction layer...

@wilbowma
Copy link
Contributor Author

wilbowma commented Oct 9, 2014

I realize the documentation for the client does point the file must be named icon.png, and maybe the abstraction is overkill. But I thought I would point it out anyway. Having a magic name just feels wrong.

At some point I did realize the `fix' I pushed was nonsense and in fact the issue I was having was caused by my directory structure.

Thanks for the clarification and thoughtful explination.

William Bowman

@elibarzilay
Copy link
Member

Well, the thing that felt wrong to me at the time is the fact that info.rkt is a template that people need to fill in-place, but I haven't found anything better that would be both customizeable and simple enough for people who are not too technical with racket. Given that you need to do that kind of template filling, the icon.rkt and the server-key file are just part of the same thing... IOW, a nicer way to let people configure it would make an icon file specification feel more natural and less of an overkill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants