Skip to content

query current location & center map#12

Open
derhuerst wants to merge 1 commit into
rastapasta:masterfrom
derhuerst:current-location
Open

query current location & center map#12
derhuerst wants to merge 1 commit into
rastapasta:masterfrom
derhuerst:current-location

Conversation

@derhuerst

Copy link
Copy Markdown
Contributor

closes #2.

@derhuerst

Copy link
Copy Markdown
Contributor Author

bump. (:

@rastapasta

Copy link
Copy Markdown
Owner

aaa will soon be back online and on the code again - thanks for the bump :)

@derhuerst

Copy link
Copy Markdown
Contributor Author

bump.

3 similar comments
@derhuerst

Copy link
Copy Markdown
Contributor Author

bump.

@derhuerst

Copy link
Copy Markdown
Contributor Author

bump.

@derhuerst

Copy link
Copy Markdown
Contributor Author

bump.

@jaller94

Copy link
Copy Markdown
Contributor

I think this is a cool feature, however, I have a couple of concerns.

  1. Mapscii gets used both locally and remotely via telnet. This feature would be solely available locally.
  2. It is platform dependent which ideally includes continuous integration tests on every targeted platform. For now the platform is simply NodeJS (and telnet).
    I do not have a MacOS device to test the feature now or in the future. It would be ideal to have an indicator whether it still works.
  3. There is a chance that Mapscii gets rejected from distribution channels, if binaries are included. In your project’s Readme you point to yet another repo, but without building steps. Is there a way to build the blob from source as part of the npm install process?
  4. The package is not published on NPM. The platform heavily invests in security audits and automated testing. It would be better to download a versioned package of the dependency from NPM.
  5. It is a convenience feature and as such might be implemented as an optional dependency. As the project already has optional dependencies, closely tied extensions could be a consideration.

Thanks so much, @derhuerst, for your contribution. I hope, we find a way to implement this without widening the technical scope of the project immensely.

@jaller94

Copy link
Copy Markdown
Contributor

I am going to merge my CoffeeScript to JavaScript PR. Sorry for the merge conflict in advance.

@derhuerst

derhuerst commented Oct 17, 2018

Copy link
Copy Markdown
Contributor Author
  1. Mapscii gets used both locally and remotely via telnet. This feature would be solely available locally.

Correct. Should we detect wether Mapscii is running inside a telnet session?

  1. It is platform dependent which ideally includes continuous integration tests on every targeted platform. For now the platform is simply NodeJS (and telnet).

There are tests for macOS and Linux.

  1. There is a chance that Mapscii gets rejected from distribution channels, if binaries are included. In your project’s Readme you point to yet another repo, but without building steps.

There are building instructions in the Homebrew formula.

Is there a way to build the blob from source as part of the npm install process?

Yes, but it basically defeats the purpose of @derhuerst/location (being portable).

  1. The package is not published on NPM. The platform heavily invests in security audits and automated testing. It would be better to download a versioned package of the dependency from NPM.

There is @derhuerst/location. Back then, I didn't have in on npm yet.

  1. It is a convenience feature and as such might be implemented as an optional dependency. As the project already has optional dependencies, closely tied extensions could be a consideration.

How would that work?

Another possible way to get the current location is via a command line option: mapscii --location $(npx location-cli --comma).

@syui

syui commented Jan 31, 2019

Copy link
Copy Markdown

@derhuerst I think "query location" is a necessary function.
But, the command line option does not work.

# mac
$ git clone https://github.com/derhuerst/mapscii
$ cd mapscii
$ npm i
$ ./bin/mapscii.sh --location `curl -sL ipinfo.io/8.8.8.8/loc`

Do you have any thoughts about what the cause might be?

@derhuerst

Copy link
Copy Markdown
Contributor Author

Another possible way to get the current location is via a command line option: mapscii --location $(npx location-cli --comma).

But, the command line option does not work.
Do you have any thoughts about what the cause might be?

I either forgot to implement it, or to mention that it would be a cool addition. 😛

@derhuerst

Copy link
Copy Markdown
Contributor Author

I have adapted my PR to the current codebase, but I'd also be okay with only having the --location CLI option.

Comment thread src/Mapscii.js
case 'c':
config.useBraille = !config.useBraille;
break;
case 'l':

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW this would conflict with the changes made in fccacad :(

@YtvwlD

YtvwlD commented Mar 7, 2020

Copy link
Copy Markdown

What about using p for "position"?

@snipem

snipem commented May 15, 2021

Copy link
Copy Markdown

Would be nice to have this feature

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

Successfully merging this pull request may close these issues.

show current location

7 participants