luiscobot commented 8 years ago

sometimes I use XO to follow tutorials and write only a few lines of javascript, and I see an extra process to use npm and install modules

IssueHunt Summary

### Sponsors (Total: $40.00) - [

issuehunt](https://issuehunt.io/u/issuehunt) ($40.00) #### [Become a sponsor now!](https://issuehunt.io/r/xojs/atom-linter-xo/issues/21) #### [Or submit a pull request to get the deposits!](https://issuehunt.io/r/xojs/atom-linter-xo/issues/21) ### Tips - Checkout the [Issuehunt explorer](https://issuehunt.io/r/xojs/atom-linter-xo/) to discover more funded issues. - Need some help from other developers? [Add your repositories](https://issuehunt.io/r/new) on IssueHunt to raise funds.

sindresorhus commented 8 years ago

It's done this way so it doesn't activate on projects using something other than XO, like ESLint or JSHint, as that would conflict. I do see your use-case though. I'm open to suggestions on how we could solve this. Maybe have a command XO: Enable that would enable XO manually? If yes, should that manual enabling persist between sessions in that project? Or anyone have a better suggestion?

// @jamestalmage @lukeed @wmertens

lukeed commented 8 years ago

A simple, single command would make sense.

Another viable option would be a 'Lint on Save' (like your esformatter package).

A possible third option would be to expose a full settings panel with:

format on save
extensions to lint
disable if .jshintrc/.eslintrc found
file patterns to ignore
custom rules (global) -- reusable rules array across projects

wmertens commented 8 years ago

How about a default XO config that is used when none is defined in the project, and a setting whether to only run XO when package.json has it?

Then you can specify your favorite things (screw semicolons, yey tabs, obviously!), and all JS files you edit can get those settings, but by default it doesn't impact you.

I suppose that setting a package.json in your home dir with an XO config would have the same effect, so pretty much that, without actually having

the file.

Wout. (typed on mobile, excuse terseness)

sindresorhus commented 8 years ago

A possible third option would be to expose a full settings panel with:

Too much. I want to keep options to a minimum.

How about a default XO config that is used when none is defined in the project, and a setting whether to only run XO when package.json has it?

:+1: Sounds good to me.

@jamestalmage You good with this?

jamestalmage commented 8 years ago

How about a default XO config that is used when none is defined in the project

XO core already has that, are we discussing something atom specific?

a setting whether to only run XO when package.json has it?

Agreed, but it should check to see if it's in package.json or locally installed (i.e. they installed without --save). If they did either, then try to run it.

wmertens commented 8 years ago

@jamestalmage but then you need to have some packages installed again, where this issue is about using the one that ships with linter-xo

On Sun, Apr 3, 2016, 3:07 AM James Talmage notifications@github.com wrote:

How about a default XO config that is used when none is defined in the project

XO core already has that, are we discussing something atom specific?

a setting whether to only run XO when package.json has it?

Agreed, but it should check to see if it's in package.json or locally installed (i.e. they installed without --save). If they did either, then try to run it.

— You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub https://github.com/sindresorhus/atom-linter-xo/issues/21#issuecomment-204841609

Wout. (typed on mobile, excuse terseness)

jamestalmage commented 8 years ago

a setting whether to only run XO when package.json has it?

....

but then you need to have some packages installed again, where this issue is about using the one that ships with linter-xo

If you are using the "only run when XO is in package.json" , I am suggesting having a local copy installed should also cause it to use XO

sindresorhus commented 8 years ago

XO core already has that, are we discussing something atom specific?

It's about being able to just put in a default XO config (same as the one in package.json) in the Atom settings.

Agreed, but it should check to see if it's in package.json or locally installed (i.e. they installed without --save). If they did either, then try to run it.

:+1:

jamestalmage commented 8 years ago

It's about being able to just put in a default XO config (same as the one in package.json) in the Atom settings.

Ahh - I see. Yeah that's cool.

How does this plugin invoke XO, (via CLI or API?) - what do we need to do in XO core to make this happen.

sindresorhus commented 8 years ago

How does this plugin invoke XO, (via CLI or API?) - what do we need to do in XO core to make this happen.

API. No changes in XO strictly required, but it would be nice to extract the logic of whether a project has XO from this plugin into XO core, so both the Atom and Sublime plugin could use it.

Sidenote: It seems this plugin already incorrectly only uses the bundled XO version instead of the local one when available. So we'll need to change it to do the same as the XO CLI.

jamestalmage commented 8 years ago

Sidenote: It seems this plugin already incorrectly only uses the bundled XO version instead of the local one when available. So we'll need to change it to do the same as the XO CLI

The issue of automatically switching for the local copy is a bit more problematic when using via API vs CLI.

The CLI switch works well because the primary consumers are humans, and humans are good at adapting on the fly. Where the CLI is used by build scripts, those are almost always packaged locally with the module anyways, and use the local CLI if they are npm run scripts. The fact that we ignore the global CLI is irrelevant to build scripts.

Doing automatic API switching is a bit more problematic. For example, if we add a method to the core API that extracts "the logic of whether a project has XO" from this module, then that method is only available in new versions of XO. If we then rely on that method from core, then this plugin breaks for old installations of XO.

I am not against the change. Just putting the possible complications out there.

sindresorhus commented 8 years ago

@jamestalmage I didn't mean we do automatic switch in the XO API. That's indeed bad. I meant we should do the same as the CLI in this plugin, not the API. Maybe expose a convenience API function in XO for achieving it.

jamestalmage commented 8 years ago

Hmm. I guess I don't understand. You are discussing automatically loading up the locally installed XO version right? That means the API this plugin uses will change.

sindresorhus commented 8 years ago

Oh, ok, I got it now. My mistake. Yeah, that would be a problem, but I think we can just enforce semver here and only support loading the same major version as this plugin bundles. So if this plugin bundles XO 1.0.0, it will only load xo@^1.0.0 locally in the user's project.

jamestalmage commented 8 years ago

Just an idea: what about creating an argv based interface. Instead of passing an options object, you pass an argv array, and get back an eslint report.

var xo = requireLocally('xo/argv-interface');

if (xo) {
  var report = xo(source, pkg, [
    '--esnext',
    '--global=foo'
  ]);
} else {
  xo = requireWithinPlugin('xo');
  x.lintFile(...); // We can use the API directly if we can't find local
}

It could share code with cli.js. I think our command line options are a lot more stable than the XO API is.

Just a thought.

jamestalmage commented 8 years ago

Or, possibly better. Let's just expose a new API method that we will support long term. You can't pass it any configuration options:

var xo = requireLocally('xo');

var report = xo.lintForIDEPlugin(filePath, fileSource);

lintForIDEPlugin would locate the appropriate package.json based on filePath, and it would always return the basic eslint report object (which is pretty stable).

I think we could commit to keeping that particular part of the API stable long term.

If we can't find the local plugin, then we are using the one packaged with the IDE plugin, and we can use the full API (because we know which version we have at that point).

sindresorhus commented 8 years ago

:+1: Good idea. Let's go with that.

Pysis868 commented 7 years ago

Sooo, tried reading this thread.

What should I do if I have a mixed project (RoR, no proper package.json file to have here) that I just want to have some (in)frequent XO linting done in Atom appear.

I have tried running the XO: Fix, Linter: Lint, Linter: Toggle, and Linter: Toggle Panel commands in Atom, and do not see the expected error status represented anywhere.

Background: Right now I have what I believe to be the linting icon towards the left of the bottom status bar telling me I have "No Issues" in my currently opened JS file (it resides in an ignored directory, if that's important), I have xo installed globally on the CLI with brew on a Mac, and running it on the file from there tells me I have a syntax error in the same file.

sindresorhus commented 7 years ago

@Pysis868 There's no support for that yet, hence why this issue is still open. It's not something I personally need, so it's waiting for someone to do a pull request adding it.

In the meantime, you could use the xo command-line tool to lint, as it works without a package.json.

Pysis868 commented 7 years ago

ok, thanks. Sorry, read #27's status and that tripped me up a bit about this issue's status.

Pysis868 commented 7 years ago

Actually, forgot to ask, I just added a sample package.json file with:

...
"devDependencies": {
  "xo": "*"
},
...

and no syntax errors (anymore!).

It still doesn't seem to be working this way either. Any tips? Is this possible to help my situation?

Pysis868 commented 7 years ago

Sorry about the posts, just want to show what stages are occurring here.

I just opened another JS file in Atom, and the XO linter panel opened with it! (Using the 'fake' package.json approach. Still don't know a better way than this one yet.)

So without creating a MCVE, I suspect the issue is that the file is excluded from source control, causing the plugin to not lint the file, and have opened issue #38 accordingly. ...

38 is closed now. atom-linter setting fixed that!

tnga commented 7 years ago

Hi ! for my point of view preview @jamestalmage 's proposal is a good idea and hope you have focused on it.

XO behavior that consist to just lint projects in which it have been explicitly defined to do the job is a really cool thing. So keeping in mine that behavior, a minimum step to enable XO to linting a project without have too include it as dev dependency can be to set a package.json like with just a XO's config mention.

// package.json
{
  xo: {}
}

This minimum requirement event for a (simple)"one file project" isn't too much comparing to use a command to enable it (which can't be very usable in case of moving/renaming 's project, or without an .xorc file or somthing like that. which do not respond to philosophy behind XO). Noting that this can also help to identify folders that files can be linting by XO event without and explicitly mention of XO as dependency.

sindresorhus commented 7 years ago

@tnga We already support "xo": false in package.json to disable XO, so I guess we could maybe support "xo": true as an indication of XO being enabled. Do realize though, that if you use this on many packages, and then upgrade XO globally, you'll have to update all these projects, or they will be failing. That's the benefit of having dependencies being local. I'm curious why you don't want XO as a local dependency?

marionebl commented 7 years ago

I think the behaviour of "xo": false is a bit surprising already. For nested projects it means "inherit", for everything else "disabled". Adding new behaviour for "xo": true would make matters worse in my opinion - how should this behave for nested projects?

I'd propose deprecating "xo": false and replacing it with something like this. "root" means the directory the given package.json is placed in.

enabled=true

{
  "xo": {
     "enabled": true
  }
}

uses defaults or config found in root/package.json
does not search further up for config

enabled=false

{
  "xo": {
     "enabled": false
  }
}

disables linting for root/**/*
does not search further up for config

enabled=inherit

{
  "xo": {
    "enabled": "inherit"
  }
}

uses defaults or config found in first root/package.json where enabled !== 'inherit'

I am not sure on merging the inherit option onto the enabled key - perhaps those should be separate?

sindresorhus commented 7 years ago

I think the behaviour of "xo": false is a bit surprising already. For nested projects it means "inherit", for everything else "disabled".

No, "xo": false just means that package.json should not be considered the root for XO and it should continue looking upwards. You can think of it like this: By default, the package.json to use is auto-detected, setting it to false forces it to be skipped, and setting it to true forces it to be used. I think that's quite simple.

With your proposal, every project would have to have "xo": { "enabled": true }, which is sub-optimal.

IssueHuntBot commented 5 years ago

@issuehunt has funded $40.00 to this issue.

Submit pull request via IssueHunt to receive this reward.
Want to contribute? Chip in to this issue via IssueHunt.
Checkout the IssueHunt Issue Explorer to see more funded issues.
Need help from developers? Add your repository on IssueHunt to raise funds.

sindresorhus commented 1 year ago

Closing as Atom is abandoned.

sindresorhus / atom-linter-xo

Add command for running without having XO installed locally or a package.json #21

the file.

38 is closed now. atom-linter setting fixed that!

enabled=true

enabled=false

enabled=inherit