Skip to content

Title feature #19 - #20

Merged
azu merged 8 commits into
azu:masterfrom
gdolle:master
May 30, 2016
Merged

azu merged 8 commits into
azu:masterfrom
gdolle:master

Conversation

@gdolle

@gdolle gdolle commented May 6, 2016 •

Copy link
Copy Markdown
Contributor

Pull-request proposal for feature #19

  • Now the title is optional
  • Pass the option title to display the file name
  • Pass a value to this previous option title:"An example of custom title" to
    display a custom title.

Bug/Known issue:

  • Does not work properly if user write asciidoc pages. See issue Code title displayed in HTML #18.
    A workaround for now is to not pass any title, and create it directly in asciidoc.
    For example
.This is the title
[import](path/to/code.ext)

Possible improvment:

  • Add code caption numbering. (would be useful for pdf format for
    example).

- Now the title is optional
- Pass the option `title` to display the file name
- Pass a value to this previous option `title:"An example of custom title"` to
  display a custom title.

Bug/Known issue:

- Does not work properly if user write asciidoc pages. See issue azu#18.
A workaround for now is to not pass any title, and create in asciidoc.
For example

```adoc
.This is the title
[import](path/to/code.ext)
```

Possible improvment:

- Add code caption numbering. (would be useful for pdf format for
  example).
Comment thread src/title.js Outdated
* @return {boolean,string}
*/
export function parseTitle(label, fileName) {
let title=[false, undefined];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Tuple mixed data type is not major in JavaScript.
It will be better that separate hasTitle (regExp.test(string) is useful) and parseTitle?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok! (with your suggestion, it is not required anymore anyway) ;)

@azu

azu commented May 6, 2016 •

Copy link
Copy Markdown
Owner

I've written another suggestion to #19 (comment)

gdolle added 5 commits May 6, 2016 20:22
- See pull request
  azu#19 (comment)
  for more details
- Asciidoc code title fixed azu#18.
- Introduce a code counter for referencing.

*Example:*

--------------------------
This is a code
[import,title:"my title",id:"ref1"](test.cpp)
and here another code
[import,title:"my title",id:"ref2"](test.cpp)
--------------------------

*Result:*

--------------------------
This is a code

> Code 0: my title
// The code ref1

and here another code

> Code1: my title
// The code ref2
--------------------------

*Cite:*

- asciidoc

"Now I cite both code <<ref1>>, <<ref2>>"

=> "Now I cite both code [Code 0], [Code 1]

NB: Here we have an automatic text replacement.

- markdown

Now I cite both code [Code 0](#ref1), [Code 1](#ref2).
@gdolle

gdolle commented May 7, 2016 •

Copy link
Copy Markdown
Contributor Author

Code updated!

@gdolle gdolle mentioned this pull request May 7, 2016
Comment thread src/parser.js Outdated
"id":undefined,
"marker":undefined
};
for(var key in keyvals) {

@azu azu May 8, 2016 •

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please use Object.keys(keyvals).forEach(key => { ... }) instead of for..in.
for...in has a trap.

for...in is not bad, but prefer use the other.

Related:

Rule guard-for-in - ESLint - Pluggable JavaScript linter

@azu

azu commented May 8, 2016 •

Copy link
Copy Markdown
Owner

@gdolle

gdolle commented May 8, 2016

Copy link
Copy Markdown
Contributor Author

Thanks for your comments. Code updated!

@azu

azu commented May 8, 2016

Copy link
Copy Markdown
Owner

It looks good :)

@azu azu mentioned this pull request May 9, 2016
3 tasks done
@prudhomm

Copy link
Copy Markdown

@azu @gdolle any news on the merging of this PR ?

@prudhomm

Copy link
Copy Markdown

titles are broken and don't look good.
Could it be possible to merge this PR and upload a new version of this plugin ?
see here http://book.feelpp.org/content/Tutorial/02-SettingUpEnvironment.html
thx in advance

@azu

azu commented May 30, 2016

Copy link
Copy Markdown
Owner

I wait for updateing #20 (comment)

titles are broken and don't look good.

Hmm, Do you use Asciidoc?
https://github.com/feelpp/feelpp-book/blob/master/Tutorial/02-SettingUpEnvironment.adoc

Previously, gitbook-plugin-include-codeblock supported only markdown, but this pull request including support asciidoc.
If it will merge, resolve your issue.

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.

3 participants