Separate server name and id, use hostname-valid encoding #216

duncanbeevers · 2017-08-07T06:05:28Z

If you name your ~/.hotel/servers files using non-uri characters, they aren't recognized by the exists middleware.

This PR allows intuitive, human-friendly naming of these files, and a more beautiful list of servers.

duncanbeevers · 2017-08-07T08:38:22Z

src/daemon/group.js

-    const id = req.params.id
-      ? this.resolve(req.params.id)
-      : this.resolve(req.hostname.replace(tld, ''))
+    const tld = new RegExp(`\\.${daemonConf.tld}$`)


The previous regexp here wasn't enforcing a dot.

duncanbeevers · 2017-08-07T08:39:03Z

src/daemon/normalize.js

+module.exports = function normalize(id) {
+  return id
+    ? punycode
+        .encode(id)


punycode won't mess with existing ASCII; it only ensures that non-ASCII characters get embedded into a hostname-valid space.

duncanbeevers · 2017-08-07T08:40:22Z

src/daemon/normalize.js

+    ? punycode
+        .encode(id)
+        .toLowerCase()
+        .replace(/[^a-z0-9-.]/g, '-')


This ensures that ASCII characters such as spaces, parentheses, commas, and so on, are encoded to a hostname-valid space.

duncanbeevers · 2017-08-07T08:41:21Z

src/daemon/normalize.js

+        .encode(id)
+        .toLowerCase()
+        .replace(/[^a-z0-9-.]/g, '-')
+        .replace(/-+/g, '-')


Because so many characters are replaced by hyphen, any run of multiple hyphens is collapsed to a single hyphen.

duncanbeevers · 2017-09-09T05:09:05Z

@typicode ping

duncanbeevers · 2018-01-11T16:11:23Z

This PR has been updated to incorporate the changes from the UI refactor.

duncanbeevers · 2018-01-11T17:13:41Z

src/app/components/Nav/index.css

@@ -69,3 +69,7 @@ li > span {
 p {
  padding: 1rem;
 }
+
+.listLink {
+  line-height: 1em;


This change prevents monitor names from collapsing.

Before

After

typicode · 2018-01-12T01:41:49Z

src/daemon/normalize.js

@@ -0,0 +1,12 @@
+const punycode = require('punycode') // eslint-disable-line node/no-deprecated-api
+
+module.exports = function normalize(name) {


Why the ternary, have you spotted a case where it can can be undefined or is it for safety?
Could you write some tests for normalize or add a comment with examples of inputs and expected outputs?

typicode · 2018-01-12T01:44:40Z

src/daemon/normalize.js

+        .toLowerCase()
+        .replace(/[^a-z0-9-.]/g, '-')
+        .replace(/-+/g, '-')
+        .replace(/-$/, '')


Not sure to understand what they're for?

.replace(/-+/g, '-') .replace(/-$/, '')

It’s to ensure that multiple invalid characters only get replaced by one -, and to remove trailing -s.

Yes, this is just for tidiness sake. In the examples I posted server names include whitespace followed by non-alpha-numeric characters resulting in double hyphens.

It looks a little cleaner to me to tidy them, but the code I wrote mostly applies to my own naming scheme. For example, it doesn't remove leading hyphens, but that's only because I'd never named a server something that would result in a leading hyphen.

I'm happy to simplify this entire section and simply use the pure punycode replacements. That seems less likely to result in id collisions where multiple similarly-named servers normalize to the same server id.

typicode · 2018-01-12T01:46:32Z

package.json

@@ -103,6 +103,7 @@
    "npm-run-all": "^4.1.2",
    "pkg-ok": "^1.0.1",
    "prettier": "^1.10.2",
+    "punycode": "^2.1.0",


Could you move it to dependencies as it's used in the daemon?

typicode · 2018-01-12T01:51:44Z

Thanks for the PR :) (and the style fix)

I'd like to understand better, the idea is to be to have "pretty" names for files and automatically get valid domain?

Something like:

~/.hotel/servers/My Server (something).json # my-server-something.localhost

What does your projects directories look like? Do they have () as well or is it just something you want to have in hotel UI?

duncanbeevers · 2018-01-12T20:12:11Z

@typicode The filenames represent the displayed values, like so:

❯ ls -1 ~/.hotel/servers/
Always Master (staging).json
Always Master.json
Auth (staging).json
Auth.json
PMP (desktop).json
PMP (tv).json
Style Guide.json
desktop (staging).json
desktop.json
hotel.json
reflector.json
tv.json

In some cases, parameterizable hotel servers would alleviate this, as many of the configurations are very similar to one another with the modification of one or two command-line flags.

…hostname

duncanbeevers force-pushed the decode-uri-component-hostname branch 2 times, most recently from c2ea9cd to a6a7c22 Compare August 7, 2017 08:29

duncanbeevers changed the title ~~Decode URI entities in id param and hostname~~ Separate server name and id, id uses hostname-valid encoding Aug 7, 2017

duncanbeevers changed the title ~~Separate server name and id, id uses hostname-valid encoding~~ Separate server name and id, use hostname-valid encoding Aug 7, 2017

duncanbeevers force-pushed the decode-uri-component-hostname branch from a6a7c22 to cb2814f Compare August 7, 2017 08:35

duncanbeevers commented Aug 7, 2017

View reviewed changes

duncanbeevers force-pushed the decode-uri-component-hostname branch from cb2814f to 0d45259 Compare August 7, 2017 15:29

duncanbeevers force-pushed the decode-uri-component-hostname branch from 0d45259 to 1a40140 Compare December 26, 2017 18:57

duncanbeevers force-pushed the decode-uri-component-hostname branch from 1a40140 to 04f3e65 Compare January 11, 2018 16:10

duncanbeevers commented Jan 11, 2018

View reviewed changes

typicode reviewed Jan 12, 2018

View reviewed changes

duncanbeevers added 3 commits January 15, 2018 20:15

Separate hostname and displayed name of servers; use punycode hostname

0a6df84

fixup! Separate hostname and displayed name of servers; use punycode …

1a00ec5

…hostname

fixup! Separate hostname and displayed name of servers; use punycode …

8813baf

…hostname

duncanbeevers force-pushed the decode-uri-component-hostname branch from 34e21c3 to 8813baf Compare January 16, 2018 04:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate server name and id, use hostname-valid encoding #216

Separate server name and id, use hostname-valid encoding #216

duncanbeevers commented Aug 7, 2017 •

edited

duncanbeevers Aug 7, 2017

duncanbeevers Aug 7, 2017

duncanbeevers Aug 7, 2017

duncanbeevers Aug 7, 2017

duncanbeevers commented Sep 9, 2017

duncanbeevers commented Jan 11, 2018

duncanbeevers Jan 11, 2018

typicode Jan 12, 2018

typicode Jan 12, 2018

j-f1 Jan 12, 2018

duncanbeevers Jan 12, 2018

typicode Jan 12, 2018

typicode commented Jan 12, 2018

duncanbeevers commented Jan 12, 2018

		@@ -0,0 +1,12 @@
		const punycode = require('punycode') // eslint-disable-line node/no-deprecated-api

		module.exports = function normalize(name) {

Separate server name and id, use hostname-valid encoding #216

Are you sure you want to change the base?

Separate server name and id, use hostname-valid encoding #216

Conversation

duncanbeevers commented Aug 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

duncanbeevers commented Sep 9, 2017

duncanbeevers commented Jan 11, 2018

Choose a reason for hiding this comment

Before

After

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

typicode commented Jan 12, 2018

duncanbeevers commented Jan 12, 2018

duncanbeevers commented Aug 7, 2017 •

edited