Page MenuHomeSolus

translate-shell
Closed, ResolvedPublic

Description

Name: translate-shell
Description: Translate Shell (formerly Google Translate CLI) is a command-line translator powered by Google Translate (default), Bing Translator, Yandex.Translate and Apertium.
Homepage: https://www.soimort.org/translate-shell/
Why should this be included in the repository? Useful for getting a quick translation without having to leave the terminal
Is it Open Source: yes, Unlicense
How many users do you anticipate will use this software? How the hell can I evaluate that? I don't even have a clue of the number of Solus users ! ;)
Link to source tarball/zip file : https://github.com/soimort/translate-shell/releases

Note: all required dependencies are already available in the repository

Event Timeline

kyrios123 created this task.Jun 8 2017, 4:10 PM
JoshStrobl triaged this task as Normal priority.Jul 12 2017, 3:32 AM
JoshStrobl moved this task from Backlog to Accepted For Inclusion on the Package Requests board.

As soon as Rocket League installs on my Windows partition I'll take a whack

RL soon Windows!? Blasphemy! Runs great on Solus ;).

I think I made a package for it last week already if you don't feel like messing with it and get caught up on playing rocket car soccer games!

Oh cool. Make sure to submit the differential! I have to play on Windows because I only have an Intel integrated graphics card :'( and it sucks on Linux for games

That's true, don't try playing on a Mac, it's horrendous!

I'll get it in in a bit when I get back to my desk.

Looks like I did build the tool on my laptop but I didn't create a package for it.

It has an odd Makefile that includes commands to create the destination directory which causes it to fail the install step(obviously don't need to create /usr/bin).

What's the best way to approach this? Create a patch for the Makefile that removes the mkdir steps or just manually run the install command from the install section of the package.yml since that's essentially all the Makefile will be doing anyways?

For reference, this is the install section of the packages "Makefile"

install: build
    @mkdir -p $(PREFIX)/bin &&\
    install $(BUILDDIR)/$(COMMAND) $(PREFIX)/bin/$(COMMAND) &&\
    mkdir -p $(PREFIX)/share/man/man1 &&\
    cp $(MANDIR)/$(COMMAND).1 $(PREFIX)/share/man/man1/$(COMMAND).1 &&\
    echo "[OK] $(NAME) installed."
JoshStrobl added a subscriber: JoshStrobl.EditedJul 12 2017, 6:43 AM

The Makefile should be patched to account for DESTDIR ($(DESTDIR)), the mkdir commands should be using install as well, and the man dir creation and file creation should be a single install command as well.

Ideally someone should upstream a proper Makefile patch, because their install recipe alone is messy and they shouldn't be calling build during it, that's what using make by itself is for.

Acutally I did package it already before I introduced the request. i'll send it right now

@kyrios123 Added a few inline comments to D592 regarding the build step that touches on what @JoshStrobl talked about hereT3851#71595

kyrios123 added a comment.EditedJul 12 2017, 7:42 AM

@rigrassm I don't see the inline comments (do not forget to click on the submit button at the bottom of the page)

I'll do the minimum for now (replace the`cp` with installfor the man pages). If you have done more work, you can update my diff with arc diff --update D592. Do not hesitate to submit the changes upstream but when I see this, I am not sure your changes will be merged.

EDIT: here is the upstream pull request

I went back and submitted the comments, still new to Phabricator and learning as I go lol.

Yeah, looking at that reply to the issue I don't think there's any chance of getting the Makefile fixed.

The only thing that I changed was removing the build step from the package.yaml and I had the espeak package set as a rundep so that the Text-To-Speech functionality would work.

I'd like to get feedback on adding the espeak package as a rundep before submitting a diff for it though since that package isn't a required for translate-shell but is in the recommended list from their README.

@rigrassm I made the requested change and I added the feedback regarding the text-to-speech in the diff. :)

After thinking about it I agree with not having a TTS package as a runtime dep. Probably isn't that widely used.

That makefile is absolutely horrendous, lol.

Appreciate you getting this package requested and added, I have to translate customer messages constantly at work and it'll be nice to have a cli too to do it with when I'm working from my personal laptop.