Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync JSON config file with command line options #58

Closed
whymarrh opened this issue Aug 16, 2014 · 10 comments
Closed

Sync JSON config file with command line options #58

whymarrh opened this issue Aug 16, 2014 · 10 comments

Comments

@whymarrh
Copy link

Idea: adding the ability to configure all the same options in the JSON config (.pa11yrc) that are available as command line flags.

I'm not 100% sure where I would draw the line on this as not all of the command line flags are worthy (e.g. help and version), but I think the others might be useful.

Thoughts? With ideas like #50/#51 and #53, maybe the JSON config is underutilised.

@whymarrh
Copy link
Author

I am willing to submit a PR.

@rowanmanning
Copy link
Member

Yes, I think this is a great idea, and would be more than willing to merge in. As to which command-line args we allow, do you think a flat list somewhere would make sense? Just to prevent use of help etc? The only downside with that is that we'd have to add new flags by name as Phantom adds them.

@whymarrh
Copy link
Author

I'm not sure I understand the questions fully, so please forgive me if I'm talking about something seemingly random.

The command line flags for pa11y can be listed out via --help and in the README file. These options can be added/removed as their command line counterparts change. What I'm thinking is that, when using the API:

var pa11y = require('pa11y');
pa11y.sniff(options, callback);

One should be able to save something resembling options to a JSON file and pass it to:

pa11y --config $filename $site

For PhantomJS arguments, I think there may be two ways to pass the options dynamically (to avoid having to keep the supported options up to date):

  1. Write out a temp JSON config file and pass just one argument:

    --config=/tmp/config.json
    
  2. Since "[the PhantomJS config file] keys are de-dashed, camel-cased equivalents of the other supported command-line options", we could just reverse that process, prepend --, and pass the arguments along.

    Something like so:

    "localStorageQuota".replace(/([a-z])([A-Z])/g, function (match, p1, p2) {
        return g + "-" + e.toLowerCase();
    });
    // "local-storage-quota"
    

I'm not sure if either of the above are particularly elegant, but they might work.

@whymarrh
Copy link
Author

whymarrh commented Oct 6, 2014

The last snippet in my comment above should actually read:

"localStorageQuota".replace(/([a-z])([A-Z])/g, function (match, p1, p2) {
    return p1 + "-" + p2.toLowerCase();
});

@whymarrh
Copy link
Author

whymarrh commented Oct 6, 2014

Any new thoughts on what's above?

@rowanmanning
Copy link
Member

Hi @whymarrh, sorry – this has been marked as unread, I've had no free time to work on pa11y and I've let the issues get ahead of me! Based on your longer reply, I think I was getting the wrong end of the stick before, and this would definitely be really useful.

Is it something you're keen to work on?

@whymarrh
Copy link
Author

whymarrh commented Oct 9, 2014

@rowanmanning no worries!

I'm definitely still willing to submit a PR for this.

@rowanmanning
Copy link
Member

Just a note, this has made it into a development version of pa11y, support for configuring any PhantomJS flag (not necessarily through a config file, but it works anyway). There's no ETA yet but I'll keep you posted.

@rowanmanning
Copy link
Member

@whymarrh can this issue close based on the new 2.0 API (where you can configure everything from the JSON config if you need to)?

@whymarrh
Copy link
Author

whymarrh commented Jun 8, 2015

Sure can!

@whymarrh whymarrh closed this as completed Jun 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants