Add Linux support #13

tschuy · 2015-09-21T01:44:51Z

Like you noted in #11, this essentially boils down to putting the app.dock into an if statement. (I don't think there's any Linux equivalent to the dock icon with a number on it, or the bouncing dock icon.)

I also rearranged the menu to be more cross-platformy, since there's no appname menu on most distributions of Linux. (Though there is in Gnome 3, the desktop manager I use, but I couldn't figure out how to add to it from Electron, and Atom doesn't use it either.)

There is a bug with desktop notifications crashing (electron/electron#2294) that seems pretty awful. The issue isn't entirely clear which versions of what it presents itself on, so I added a note to the README about it. The notifications themselves work just fine, and as long as you don't click on them, the system won't crash.

I would be happy to make any changes you need before merging this. I could also test it on a non-Gnome environment.

sindresorhus · 2015-09-21T04:26:24Z

menu.js

+		}
+	}
+);
+


Since the menues are so different on OS X and Linux I think it would be better to just create a separate file with a Linux menu instead of juggling this mess. The Help menu is common and can be a menu-common.js file and required into each.

That's reasonable. It started out not too bad, and then as I added more and more to it, it got crazier and crazier 😛

sindresorhus · 2015-09-21T04:31:13Z

Make sure the Linux menu is consistent with other Linux apps.

sindresorhus · 2015-09-21T04:33:14Z

menu.js

+		type: 'separator'
+	},
+	{
+		label: 'Preferences...',


Re #13 (comment)

On Linux it's only Preferences, not Preferences....

tschuy · 2015-09-21T05:04:02Z

Unfortunately, About goes under help on Linux, so I can't actually factor out the help menu. 😢

From what I managed to get by messing around with electron-packager the best I'll be able to do is make an executable that can then be zipped up, it doesn't have a way to create a single runnable file. I'll make the script throw the build into a zip.

tschuy · 2015-09-21T05:13:50Z

what would you prefer the behavior of npm run build be? it's constrained by running /bin/sh, not /bin/bash, so I can't do something fancy like if [[ "$OSTYPE" == "linux-gnu" ]]; then npm run build-linux; else npm run build-osx; fi. I could write a separate build.sh script that compiles your current system, or I could just make it run OS X by default, or both?

sindresorhus · 2015-09-21T05:24:07Z

npm run build should build for both platforms. electron-packager can handle this fine. See its docs.

tschuy · 2015-09-21T06:10:17Z

When trying to build them together, I run into the following problems:

sign doesn't exist on Linux, but electron-packager still tries to use the codesign executable, so it crashes
it doesn't appear to want to put the executable into an archive, even when -asar is set
for whatever reason, instead of setting the app icon at compile time, it's set in the source code as the icon value passed to BrowserWindow. However, media/ isn't included in the build, so there's no static file to reference.

I think two separate build commands might be necessary for that top reason. For the bottom two, I'm sure it's just me being silly and missing something in their documentation. (I still have no idea what asar is supposed to look like when it's successfully outputted.)

sindresorhus · 2015-09-21T06:27:01Z

sign doesn't exist on Linux, but electron-packager still tries to use the codesign executable, so it crashes

Ugh, electron-packager is so buggy. Wish we had something better...

I'm ok with just being able to build from OS X. I'm the only one that creates builds anyways. Having one build massively simplifies it, so I think it's worth the limitation.

it doesn't appear to want to put the executable into an archive, even when -asar is set

Can you elaborate? It's the zip command that does this, not electron-packager.

for whatever reason, instead of setting the app icon at compile time, it's set in the source code as the icon value passed to BrowserWindow. However, media/ isn't included in the build, so there's no static file to reference.

The media folder is ignored. I guess we can ignore everything in the media folder, except the icon.

tschuy · 2015-09-21T06:37:11Z

Can you elaborate?

from electron-packager's docs, it reads like asar should put everything into an archive: https://github.com/maxogden/electron-packager#usage

for now, I've just set the build-linux command to stick it into an archive itself.

What I could do is have build-linux and build-osx commands, and just have build be npm run build-osx && npm run build-linux. This would still be one command, and it'd allow anyone that wants to run builds themselves (though you're right that people working on the project have no reason to build, only you) to just run the build they want. I don't know if this would break some caching mechanism that electron-packager has, though.

I tried removing media/ from the build command for Linux and it's not included in the dist/Caprine-linux-x64 folder, so I'm going to look more into it.

tschuy · 2015-09-21T07:15:28Z

ok, I have a solution that more or less works, as far as I can tell. It should allow you to create a Linux zip and an OS X zip by running build, and allow others to create zips for either system by running build-osx or build-linux. The Linux build has a functioning icon, and I included a .desktop template in the README for anyone who wants to create a shortcut. Since this isn't using a proper install mechanism (which is to say, a deb or rpm), installation is a little clunky.

sindresorhus · 2015-09-21T09:07:46Z

menu.js

-${app.getName()} ${app.getVersion()}
-${process.platform} ${process.arch} ${os.release()}`;
+		${app.getName()} ${app.getVersion()}
+		${process.platform} ${process.arch} ${os.release()}`;


Don't indent this. Template literals are whitespace sensitive.

sindresorhus · 2015-09-21T09:17:02Z

menu.js

+					}
+				},
+				{
+					label: `About ${appName}`,


On Ubuntu it's just About.

Look at the existing Linux menues to make sure everything is consistent.

sindresorhus · 2015-09-21T09:20:11Z

Is there any way to have the app installable on Linux with something like Homebrew Cask on OS X?

If https://github.com/loopline-systems/electron-builder supported Linux we could use that. Maybe worth opening a feature request for that?

adam-stokes · 2015-09-21T12:12:06Z

We could look at how atom builds their deb's/rpm's and mimic that. Would at least support the major distributions

tschuy · 2015-09-25T03:09:30Z

I've addressed your feedback about the menus and fixed the accidental inclusion of linux in the build-osx command.

For menu consistency, I'm not sure there's a really good standard compared to what there is on OS X. I tried to go off of Gedit. Most Gnome apps use the menu at the top of the screen (that I've not seen any other app use, and don't think there's support for in electron), but gedit has a more traditional File / Edit / Help etc.

You mentioned that the edit menu shouldn't have some of those items, but gedit has all of them. I've left them there for now; which ones were you thinking of?

About packaging -- I don't think there's a good way to distribute this unless we make it build an RPM and a DEB. I'll make a feature request on https://github.com/loopline-systems/electron-builder, and revisit this later if they add support.

tschuy · 2015-09-25T03:09:56Z

Also, I'd be more than happy to squash these commits if you want so that address feedback isn't in the git log.

tschuy · 2015-09-25T03:10:27Z

issue for adding linux support to the builder: electron-userland/electron-builder#7

adam-stokes · 2015-09-25T03:17:41Z

You mentioned that the edit menu shouldn't have some of those items, but gedit has all of them. I've left them there for now; which ones were you thinking of?

I think it just depends on the application, things like gedit, kate, mousepad, etc make use of those edit menu items where as others don't. They probably are useful for any type of text editing but I'm not sure this app needs menu items for it. Take for example Telegram on the desktop for Linux, it doesn't contain any of those menu items (File, Edit) etc.

sindresorhus · 2015-09-25T07:45:21Z

@tschuy Landed! Really appreciate you working on this :)

tschuy force-pushed the master branch from ff1e3f4 to 12a1443 Compare September 21, 2015 01:48

Add Linux support

e4bc5da

tschuy force-pushed the master branch from 12a1443 to e4bc5da Compare September 21, 2015 02:10

sindresorhus reviewed Sep 21, 2015
View reviewed changes

clean up menu code and fix readme

76808ed

build-linux capability

cf3fc5c

fix packaging

1ea6fc6

tschuy force-pushed the master branch from 995d332 to 1ea6fc6 Compare September 21, 2015 07:16

sindresorhus reviewed Sep 21, 2015
View reviewed changes

address feedback

499e679

tschuy force-pushed the master branch from 5ddfa65 to 499e679 Compare September 25, 2015 03:03

sindresorhus closed this in d0da181 Sep 25, 2015

sindresorhus mentioned this pull request Sep 29, 2015

Linux support #1

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Linux support #13

Add Linux support #13

tschuy commented Sep 21, 2015

sindresorhus Sep 21, 2015

tschuy Sep 21, 2015

sindresorhus commented Sep 21, 2015

sindresorhus Sep 21, 2015

tschuy commented Sep 21, 2015

tschuy commented Sep 21, 2015

sindresorhus commented Sep 21, 2015

tschuy commented Sep 21, 2015

sindresorhus commented Sep 21, 2015

tschuy commented Sep 21, 2015

tschuy commented Sep 21, 2015

sindresorhus Sep 21, 2015

sindresorhus Sep 21, 2015

sindresorhus commented Sep 21, 2015

adam-stokes commented Sep 21, 2015

tschuy commented Sep 25, 2015

tschuy commented Sep 25, 2015

tschuy commented Sep 25, 2015

adam-stokes commented Sep 25, 2015

sindresorhus commented Sep 25, 2015

Add Linux support #13

Add Linux support #13

Conversation

tschuy commented Sep 21, 2015

sindresorhus Sep 21, 2015

Choose a reason for hiding this comment

tschuy Sep 21, 2015

Choose a reason for hiding this comment

sindresorhus commented Sep 21, 2015

sindresorhus Sep 21, 2015

Choose a reason for hiding this comment

tschuy commented Sep 21, 2015

tschuy commented Sep 21, 2015

sindresorhus commented Sep 21, 2015

tschuy commented Sep 21, 2015

sindresorhus commented Sep 21, 2015

tschuy commented Sep 21, 2015

tschuy commented Sep 21, 2015

sindresorhus Sep 21, 2015

Choose a reason for hiding this comment

sindresorhus Sep 21, 2015

Choose a reason for hiding this comment

sindresorhus commented Sep 21, 2015

adam-stokes commented Sep 21, 2015

tschuy commented Sep 25, 2015

tschuy commented Sep 25, 2015

tschuy commented Sep 25, 2015

adam-stokes commented Sep 25, 2015

sindresorhus commented Sep 25, 2015