Some code improvements + unit test reproducing an unsolved bug. #3

wants to merge 9 commits into


None yet

4 participants

flavio commented Sep 3, 2010

Hello I fixed a couple of small issues:

I also found a rendering issue that I have not been able to fix. This is highlighted with a unit test provided by commit flavio/terminal-table@57615ad. Unfortunately I have not found a solution yet.

flavio added some commits Aug 6, 2009
@flavio flavio added colspan support to table 973743f
@flavio flavio updated examples 4eb6771
@flavio flavio updated unit tests 4a94ba6
@flavio flavio updated readme 99add81
@flavio flavio fixed typo 8a0cb54
@flavio flavio Merge remote branch 'visionmedia/master' into integration
* visionmedia/master: (35 commits)
  Release 1.4.2
  Added more examples
  Misc refactoring to last guys contrib
  Misc refactoring to last guys contrib
  Modified the tests to correct several bugs with cells that span multiple columns
  Release 1.4.1
  Fix column alignment with separators.
  Revert "Add failing spec for bug which I introduced :("
  Add failing spec for bug which I introduced :(
  Release 1.4.0
  Fixed invalid example
  Updated examples
  Inline comments for #render_cell
  Refactoring #render with array buffer
  Misc refactoring
  No need for a Separator class. Just use a symbol.
  Render custom separators with Table#add_separator.
  Fix common typo: seperator -> separator.
  Release 1.3.0
  Misc refactoring

@flavio flavio consider Y length when calculating column width e6af300
@flavio flavio found a layout error 57615ad
@flavio flavio improve border less tables 658609b
tj commented Sep 9, 2010

does not seem to apply cleanly to master

flavio commented Sep 16, 2010

What error do you get? Everything is working fine here. This is what I did to reproduce your issue:

  • I cloned your repo
  • git remote add flavio my_repo_url
  • git fetch flavio
  • git co -b integration
  • git merge flavio/master

Did I miss something?

tj commented Sep 16, 2010

weird i will have to try again

tj commented Sep 17, 2010

Applying: added colspan support to table
error: patch failed: lib/terminal-table/cell.rb:3
error: lib/terminal-table/cell.rb: patch does not apply
error: patch failed: lib/terminal-table/heading.rb:2
error: lib/terminal-table/heading.rb: patch does not apply
warning: lib/terminal-table/table.rb has type 100755, expected 100644
error: patch failed: lib/terminal-table/table.rb:58
error: lib/terminal-table/table.rb: patch does not apply
Patch failed at 0001 added colspan support to table
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

sorry i would usually just merge, but I have tons of projects to maintain

flavio commented Sep 17, 2010

sorry i would usually just merge, but I have tons of projects to maintain

I totally understand you, don't worry.

How can I reproduce it, are you performing a git rebase?

tj commented Sep 18, 2010

nope, just my usual curl | git am

flavio commented Sep 27, 2010

I tried to apply the patch in the same way but I have not been able to find a solution.
Could you please apply it using the following one-line command?

Just execute it inside of your local working directory:

git remote add flavio git:// && git fetch flavio && git co -b integration && git merge flavio/master

Once the code has been executed you will find yourself into the integration branch.

git diff master

will show you all the changes I made.

Thanks a lot for your time.

rrrene commented Oct 17, 2011

Unfortunately, even if the patch would apply, there are some errors in your spec's fixture string (the row numbering starts at 1 and not zero and the third heading is aligned right not left).

But it's a good spec so I fixed it and appended it to this pull request: #13


Closing in favor of #13.

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