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

Improve recipe export (text) & add BBCode export #66

Merged
merged 1 commit into from Oct 5, 2015

Conversation

Projects
None yet
2 participants
@theophae
Copy link
Contributor

theophae commented Jul 31, 2015

I changed almost all the behavior of the getTextFormat function in order to make it smaller and easier to maintain. I split this function in order to reuse all sub-function for BBCode extraction.
I change many thing to use the same unit and scales as in the main interface (it seems more logical).
Doing this I found a bug on the displayAmount function which was trashing the precision parameter in some cases.
I think that it can still be improved, but it is quite OK now.
I didn't give an implementation for the BBCode function as it would be considered as a new feature (even if I'd really like to see it included into 2.2.0), but it will be very easy to write know, as it will be base on the text format extract.
One other thing I did, is to clean the calories management. Before that it was sometime per 33cl and other times per 35cl.

@Brewtarget/developers : Please review and give me some feedbacks

@rocketman768

This comment has been minimized.

Copy link
Member

rocketman768 commented Aug 1, 2015

Great! Reviewing...

@rocketman768

This comment has been minimized.

Copy link
Member

rocketman768 commented Aug 1, 2015

Looks like good code. Two suggestions for the text format:

  1. "srm" should be capitalized to "SRM"
  2. The units should be aligned vertically with each other somehow

I'm glad we can finally have BBCode at some point 😄

@theophae

This comment has been minimized.

Copy link
Contributor

theophae commented Aug 1, 2015

Regarding the point 1, I agree. I think that we must change several unit display:
sg -> SG
ebc -> EBC
srm -> SRM

I'd also like to make units translatable. The best example is "day" that stays in English in every languages.
But I think that theses changes should be done in another branch.

I also agree about the unit alignment, I try to do something about it.
Do you want me to add BBCode to this pull request ? Because right now it will be nothing to develop.

@rocketman768

This comment has been minimized.

Copy link
Member

rocketman768 commented Aug 1, 2015

Sure; go ahead with the BBCode.

@rocketman768

This comment has been minimized.

Copy link
Member

rocketman768 commented Aug 1, 2015

I just noticed the refractometer tools no longer allow fractional input. On the input side, all of the fractional parts of the field get rounded to a single digit. For example "7.87 P" becomes "8 P" and "1.045 sg" become "1 sg".

@theophae

This comment has been minimized.

Copy link
Contributor

theophae commented Aug 1, 2015

I've notice some similar bugs. But I know why it appends.
I've corrected a bug that trashed the precision asked for the displayed value. So everywhere the precision is 0 it is now taken into account. I need to check every displayAmount call to see if the precision passed is correct. By the way, I think that a precision of 3 is too big in most of the case. It would be better to pass it to 2.
@mikfire : If I misunderstood something about the unit system please tell me.

@theophae

This comment has been minimized.

Copy link
Contributor

theophae commented Aug 1, 2015

I've fixed the bug on gravity and color display. You can check if you see other bugs.

@theophae

This comment has been minimized.

Copy link
Contributor

theophae commented Aug 1, 2015

I've added the BBCode export, but I don't know how to connect it to the interface. I thought of two solutions:

  • To add an option to choose between plain text format or BBCode format using the current button
  • To add a new menu entry for it

What do you prefer ?

@rocketman768

This comment has been minimized.

Copy link
Member

rocketman768 commented Aug 1, 2015

Gravity bug looks fixed.

I prefer another menu entry for BBCode export. It should probably be in the File->Recipe menu.

@theophae

This comment has been minimized.

Copy link
Contributor

theophae commented Aug 1, 2015

Now you can export to BBCode from File->Recipe.
I've also improved instruction formatting for multiple line instructions.

I didn't change the alignment on the unit as rocketman768 asked because it is not trivial because the amount and its unit come directly together from displayAmount function.
If anyone see a simple way to do it, please tell me.

Enjoy BBCode ;)

@rocketman768

This comment has been minimized.

Copy link
Member

rocketman768 commented Aug 1, 2015

Thanks @theophae! If nobody else has any comments, go ahead and squash these commits.

@theophae theophae force-pushed the theophae:bugs/recipeExportScale branch from 26b8ef5 to 3d69cfd Aug 2, 2015

@theophae

This comment has been minimized.

Copy link
Contributor

theophae commented Aug 2, 2015

I've change the BBCode export to set tables' first line to bold (I wanted to do it at start but I've just figured out how to do it properly).
I squashed all commits so you can merge it.

@theophae theophae force-pushed the theophae:bugs/recipeExportScale branch from 3d69cfd to 3fce8de Aug 2, 2015

@theophae theophae changed the title Improve recipe extraction Improve recipe export (text) & add BBCode export Aug 3, 2015

@theophae theophae force-pushed the theophae:bugs/recipeExportScale branch 2 times, most recently from ad7d3b1 to 644db81 Aug 11, 2015

@theophae

This comment has been minimized.

Copy link
Contributor

theophae commented Aug 11, 2015

As it hasn't been merge yet, I've added a function to wrap too long instruction strings to keep most of the text below the 80 chars limit.
I think that we should rewords some of the instruction (as the "Add grain" instruction) which are not very clear. But it will be in another pull request.
Unless you ask me to change some points, I won't make any more change ;-)

@rocketman768 rocketman768 referenced this pull request Sep 22, 2015

Closed

2.2.0 Stable Branch #44

@theophae theophae force-pushed the theophae:bugs/recipeExportScale branch 3 times, most recently from 6931b5f to 9df9fdb Sep 22, 2015

@theophae theophae force-pushed the theophae:bugs/recipeExportScale branch from 9df9fdb to 6d96caf Oct 4, 2015

rocketman768 added a commit that referenced this pull request Oct 5, 2015

Merge pull request #66 from theophae/bugs/recipeExportScale
Improve recipe export (text) & add BBCode export

@rocketman768 rocketman768 merged commit bb879b9 into Brewtarget:master Oct 5, 2015

@theophae theophae deleted the theophae:bugs/recipeExportScale branch Nov 12, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment