Skip to content

Rightalign numeric values #105

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

Merged
merged 3 commits into from
Mar 15, 2015

Conversation

muhqu
Copy link
Member

@muhqu muhqu commented Feb 15, 2015

addresses #85

The Pretty Printing was done via gherkin.formatter.PrettyFormatter which is part of cucumber/gherkin. This pull request copied the original PrettyFormatter and added the missing feature to right-align numeric values in tables.

screen shot 2015-02-15 at 23 51 56

@muhqu
Copy link
Member Author

muhqu commented Feb 15, 2015

I just noticed that upgrading the gherkin lib to 2.12.2 was not needed, as I ended up copying the PrettyFormatter anyway...

@muhqu muhqu force-pushed the rightalign-numeric-values branch from aae3671 to 6ac70b4 Compare February 16, 2015 08:18
@muhqu
Copy link
Member Author

muhqu commented Feb 16, 2015

I rebased the PR to remove the unnecessary gherkin lib upgrade.

@ilanpillemer
Copy link
Member

I am a bit uncomfortable with duplicating the code from Gherkin. We should just just use the Gherkin library, @aslakhellesoy - any thoughts?

@muhqu
Copy link
Member Author

muhqu commented Feb 16, 2015

I wanted to subclass the pretty formatter to add the missing features but its implementation relies heavily on private fields and methods which make it hard to add these.

It would make sense to apply the changes I made to the pretty formatter also to the gherkin lib. It is just that I don't like to have to wait for them to review,merge and release these missing features until I can use them were I need them... ;-) if they accept my pull requests, I'll be happy to switch back to the original gherkin lib.

The pretty formatter is very much self contained. E.g. a single class file.

@muhqu
Copy link
Member Author

muhqu commented Feb 17, 2015

@ilanpillemer FYI: I just submitted a PR for the gherkin lib to add some formatting options I want to add to cucumber-eclipse... cucumber/gherkin#331

@muhqu
Copy link
Member Author

muhqu commented Feb 17, 2015

💡 it might be nice to allow users to configure a command-line formatter... I've written such a tool in golang: it's name is gherkinfmt and it uses my go-gherkin package...

@muhqu muhqu mentioned this pull request Feb 24, 2015
4 tasks
@muhqu
Copy link
Member Author

muhqu commented Feb 24, 2015

Added another PR #107 which supersedes this one... still leaving this one open, as it could still be merged independently... :shipit:

@ilanpillemer ilanpillemer merged commit 6ac70b4 into cucumber:master Mar 15, 2015
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.

2 participants