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

Table valign #321

Closed
wants to merge 7 commits into from
Closed

Table valign #321

wants to merge 7 commits into from

Conversation

Skeeve
Copy link
Contributor

@Skeeve Skeeve commented Dec 31, 2013

Fixing issue #314 and issue #315
Adding valignment using ^ and ,
Adjusting the documentation

@jeremy: check the top of the ".tid" file as I did not change the metadata of the documentation tiddler.

@willover
Copy link
Contributor

willover commented Jan 1, 2014

Fast work @Skeeve! wd!

There are some further thoughts regarding the use of extra extension formatting characters for tables. These relate to @Jermolene's suggestion to consider multiline tables at the last Hangout (30th Dec).

Please could we hold off committing to the latent documentation of table syntax until we can discuss the options at the next Hangout (7th Jan)?

@Skeeve
Copy link
Contributor Author

Skeeve commented Jan 2, 2014

You'd better ask Jeremy.

@willover
Copy link
Contributor

willover commented Jan 2, 2014

Hi @Skeeve. The comment was destined for Jeremy in case he plans any pulls; of course, he'll automatically see it as a result of having being tagged @Jermolene.

@@ -64,6 +76,16 @@ var processRow = function(prevColumns) {
// Look for a space at the start of the cell
var spaceLeft = false,
chr = this.parser.source.substr(this.parser.pos,1);
var vAlign = null;
if (chr === "^") {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Skeeve there's a few coding style inconsistencies. I'll clean them up but would appreciate attention to them - thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am 03.01.14 11:34, schrieb Jeremy Ruston - notifications@github.com:

In core/modules/parsers/wikiparser/rules/table.js:

@@ -64,6 +76,16 @@ var processRow = function(prevColumns) {
// Look for a space at the start of the cell
var spaceLeft = false,
chr = this.parser.source.substr(this.parser.pos,1);

  •       var vAlign = null;
    
  •       if (chr === "^") {
    

Hi @Skeeve https://github.com/Skeeve there's a few coding style
inconsistencies. I'll clean them up but would appreciate attention to
them - thank you!


Reply to this email directly or view it on GitHub
https://github.com/Jermolene/TiddlyWiki5/pull/321/files#r8631814.

Could you please tell me what was inconsistent? I thought everything was
fine.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see the changes I made in these two commits:

1374bd9
8fc5c1d

Minor stuff, it's not a big deal to fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay… So there are additional spaces around operators and no spaces
around kommas, right?

I hope the try-catch is not in the final release? I still see it in
1374bd9
1374bd9

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try/catch is fixed in 8fc5c1d

Jermolene pushed a commit that referenced this pull request Jan 3, 2014
Jermolene pushed a commit that referenced this pull request Jan 3, 2014
Jermolene pushed a commit that referenced this pull request Jan 3, 2014
@Jermolene Jermolene closed this Jan 3, 2014
@Jermolene Jermolene mentioned this pull request Jan 3, 2014
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.

None yet

3 participants