CVS edit link for verbosity

Building several Drupal modules in my workplace, would like to contribute them to the community and get feedback for them as well.

The main project I'm currently working on is a module to allow the export of books in various document formats, epub being the main one, but also including DTBooks ( DAISY digital talking books, for use in systems used by those with forms of visual disability ).

ongoing goals include outputing .pdf, and allowing the parsing of html(inc. word html)/xhtml into books.

I've also created a small module to enhance the use of context help.

These have been built for drupal 6

CommentFileSizeAuthor
#35 freetts0.9.zip7.01 KBverbosity
#33 freetts0.86.zip10.1 KBverbosity
#28 freetts0.85.zip11.18 KBverbosity
#27 freetts0.8.zip11.06 KBverbosity
#25 Freetts0.7.zip11.24 KBverbosity
#26 Freetts0.7.5.zip10.97 KBverbosity
#24 Freetts.zip11.29 KBverbosity
#8 epub module.zip14.91 KBverbosity
#2 epub.zip9.67 KBverbosity

Comments

verbosity’s picture

verbosity commented

Just as an update ( apologies maybe I should've waited till my module was nearly finished, there should be something functional to show by friday.

my main focus is a module ( or possibly more than one), that converts drupal books into ereader friendly formats ( specifically epub and DTBooks ) I shall have a (semi)functional version of this within a day or so.

In the last day I've also realised that I'll also need to build an additional module (originally conceived as being included in the epub module), wich will take several formats of documents ( html, word, epub, DTBook etc) and convert them into books.

for clarification, I use book in the book module sense and hope in both cases that the structure will be kept and there will be a good level of semantic-ness to these conversions.

I don't expect the status to be changed, but have to admit I didn't read the instructions properly before applying.

verbosity’s picture

verbosity commented
Status: Needs review » Postponed (maintainer needs more info)
StatusFileSize
new epub.zip9.67 KB

Please find attached the first draft of the epub module.

While this is functional this is an early draft and probably breaks a few coding standards ( as well I'm sure rules for file management ), although that is partly the reason for me applying ( to get some feedback + possibly assistance ).

install Instructions:
extract the zip and copy the module to your sites module directory. copy the epub_files folder ( and contents ) to the base dir of your drupal site.

instructions for use:
This module is a plug-in for book, it supplies a link at the bottom of each book node which upon clicking creates an epub document.
epub is the standard format for many ebook readers. the book page must be plain text or filtered html.

Issues ( things I'm aware of and am already working on):
1. files probably created/placed/kept in a bad place.
2. page that is displayed after creation is a debug page
3. no link provided ( some sort of a list of these files is needed to allow easy download)

TODO's:
all the issues listed above,
add DTBook support ( similar format to epub for ereaders designed for those with visual disabilities)
probably other stuff that I"m not aware of.

-----------------------------------------------------------------------
what is the epub module?

Currently drupal has systems for exporting books ( and book nodes) to html ( printer friendly ) and pdf. formats. But there are many other formats that are available and used both on standard computing devices as well as mobile platforms, ereaders and devices for those with accessibility issues.
I was tasked to create a system to easily create epub format documents, and this was extended to DTBooks since the formats are similar.

how does this compare to other modules?
While drupal can create html and .pdf format documents there is currently no core or contrib features that support epub or DTBook.

How does the epub module work?
simply put, the epub module collects the nodes in a book, organises them and packages them up in an epub file. This functionality can be tested in the current module and the resultant file can be viewed in any supporting ereader ( firefox also has a plug-in for viewing this)
-----------------------------------------------------------------------

My motivation:

I've recently been employed to work with drupal systems, and have created a few 'mods' for drupal modules, and would like to offer what I have done/will do to the community. CVS access isn't something that interests me in itself, but the ability to post my module(s) for feedback/use by others is something important to me.

verbosity’s picture

verbosity commented
Status: Postponed (maintainer needs more info) » Active
verbosity’s picture

verbosity commented
Status: Active » Needs review

oops wrong one, sry

joachim’s picture

joachim commented
Status: Postponed (maintainer needs more info) » Needs work

Hi.

Thanks for your application.

There is an existing ePub module here: http://drupal.org/project/epub

Project duplication is something we avoid in Drupal. Could you investigate this module to see if and how it differs from yours? You are encouraged to work with existing projects rather than create projects that duplicate functionality.

verbosity’s picture

verbosity commented

Hi,
When I started to do this module I did a search and came up with no results ( the epub project was not published at that time), hence my belief that there was no other module.

I'll be happy to check out the module you mentioned and work within it if at all possible.

verbosity’s picture

verbosity commented
Status: Needs work » Closed (won't fix)

having no module to submit I currently have no need of cvs access.

verbosity’s picture

verbosity commented
Status: Closed (won't fix) » Active
StatusFileSize
new epub module.zip14.91 KB

re-opening my case, i hope to detail my reasons below.

1. My module publishes to DTBooks format as well as Epub
2. IMCE based image support where included
3. fully integral system, no need for additional modules / libraries

In addition to that, I have tried to contact the maintainer to talk to them about their module and got no response also I posted a bug for the said module ( because it does not work - it does not actually create epub documents ), and that has not been replied to either.

My module has improved significantly since I last posted it, I hope to have something more 'finished' byt the end of the week. However, here is a more up to date 'development' version.

To be honest I have no real want of CVS access ( though the ability to make the code I've written is nice, as would the ability to create project pages), But I'm developing this for my work anyway, and I believe it to be a better module ( though I accept that I'm biased)

joachim’s picture

joachim commented
Status: Active » Needs work

> In addition to that, I have tried to contact the maintainer to talk to them about their module and got no response also I posted a bug for the said module ( because it does not work - it does not actually create epub documents ), and that has not been replied to either.

Could you post the link to that issue please? It doesn't look like that module has been abandoned -- last commit was 2 weeks ago. Maybe give the maintainer a bit more time to respond?

verbosity’s picture

verbosity commented

http://drupal.org/node/894320

Sometimes I don't come across as I intend, It was not my intention to be disparaging toward the other module, and I didn't suggest that the module had been abandoned.

Maybe give the maintainer a bit more time to respond?
I'm afraid my development is work related, and as such I have a deadline to meet, hence why my development of this has continued ( and will continue... )

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

Hello, and thanks for applying for a CVS account.

I would not call abandoned a project that have been created on August 18; if the maintainer is not replying is because he is probably in vacation.
I am not sure I understand the purpose of this CVS application; is it to add a new module in Drupal.org repository, or to become co-maintainer of an existing project?

Everett Zufelt’s picture

Everett Zufelt commented
Issue tags: +Accessibility

I am quite interested in the ability to export as DAISY. Can you please provide some more detail about type and version of DAISY supported?

Thanks

joachim’s picture

joachim commented

> I am not sure I understand the purpose of this CVS application; is it to add a new module in Drupal.org repository, or to become co-maintainer of an existing project?

That depends on what the other module maintainer replies!

joachim’s picture

joachim commented

> Sometimes I don't come across as I intend, It was not my intention to be disparaging toward the other module, and I didn't suggest that the module had been abandoned.

@verbosity: that's maybe not the best way to have started communication with the other module's maintainer... also, file one issue per problem. So one for the crash ;) and one to discuss merging your work.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: -Accessibility +Module review
verbosity’s picture

verbosity commented
Issue tags: -Module review +Accessibility

@Everett Zufelt
I'm having a little difficulty to get a functioning DAISY document yet ( it's almost there, but not quite ), my goal is to be be able to output both DAISY 2 (2.02 I believe ) and DAISY 3. I should have an update to my work that includes DAISY in a day or two. These wont by default include text to speech ( however if I can find something to plug in that might do the job I'd have a look at it ), however DAISY3 documents will be able to incorporate associated audio files ( and possibly support recording of files). At the moment I'm at the stage where I have code that is generating DAISY documents, but I'm unable to test them at work so development is a little slower than I would like.

@joachim
I have also sent a message to the modules maintainer. however I'm building my own module as it stands anyway ( its for work, and I therefore have a fixed set of goals and a time constraint ), though I would be willing to put my code to work in the existing module if the maintainer was willing.

Edited to expand on the reply to Everett.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
  • The points reported in this review are not in order or importance / relevance.
  • Most of the times I report the code that present an issue. In such cases, the same error can be present in other parts of the code; the fact I don't report the same issue more than once doesn't mean the same issue is not present in different places.
  • Not all the reported points are application blockers; some of the points I report are simple suggestions to who applies for a CVS account. For a list of what is considered a blocker for the application approval, see CVS applications review, what to expect. Keep in mind the list is still under construction, and can be changed to adapt it to what has been found out during code review, or to make the list clearer to who applies for a CVS account.
  1. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how the code should be formatted; how Drupal variables, global variables, constants, and functions defined from the module should be named. 
  2.       $output = '<p>'. t('The book module ') .'</p>';
      $output .= '<p>'. t('Pages in the book hierarchy .', array('@admin-block' => url('admin/build/block'))) .'</p>';

    The call to t() is not correct; the string is not using any placeholders contained in the second argument. 

  3. 	$epub_get_title_query=db_query( ' SELECT link_title 
									FROM  `menu_links` 
									WHERE menu_name =  "book-toc-'.$book_id.'"
									ORDER BY p1, p2, p3, p4, p5, p6, p7, p8, p9 ASC LIMIT 1' );

    The query should use placeholders.

  4. Remove any debug code, even the commented one. 
  5. /** cheap copy of book module, exras remved and needed areas modified for epub
* this is my first attempt at a drupal module so will be heavily annotated
*
*structure
*1. main system hooks ( menu,help, etc)
*2.supplementary 'book' based code ( code straight from or only slightly altered from book )
*3. main system functions - custom functions vital to epub / etc /
*/


    /*
*xhtml_header_epub
*
*takes raw node information and  turns it into xhtml
*/

    The comments don't follow the format reported in the coding standards, and contains some typos. 

  6.           'attributes' => array('title' => t('export the contents of this book into ereader friendly epub format'))

    The title should start with a capitalized word, and end with a period. 

  7. /**
* note based on  book.pages.inc
*credit to the author for the inspiration
*other wise copyright IRISS
*
*released under GPLv2.0 or later
*

    Modules that are included in Drupal.org repository are licensed under the same license used by Drupal; any reference to the license should be avoided, inside the code.
    I am not sure what you mean by other wise copyright IRISS. 

  8. $epub_path=$_SERVER['DOCUMENT_ROOT'].'/epub_files/';

    To what is that path supposed to point? 

  9. 	$book_query_reference=db_query( ' SELECT book.bid, node.title FROM book, node WHERE book.nid='.$nid.' AND book.nid=node.nid LIMIT 1');

    Use db_query()-placeholders; for queries that needs to limit the number of returned rows, there is a specific Drupal function.

  10. Instruction files should be plain text.
  11. The directory epub_files needs to be removed.
  12. The name of the module needs to be changed; there is already a project that is used epub as short name; that is the reason I was not understanding if the application was done to be co-maintainer or to create a new project.
verbosity’s picture

verbosity commented

thanks for the feedback,

a couple of specific comments in regards to it.

7. noted, I'll remove the licence comment, IRISS is the charity I work for, however its not a problem to remove that note.

11. removed or moved? would it be acceptable for it to default into the /site/files directory? I plan to integrate file management into Drupal as far as i possibly can( on my todo )

12. agreed

most of the other comments were helpful and I'll work in improving the code to get it up to standard. Its worth pointing out that I'm still spending quite a bit of time working on this daily and my goal has been functionality rather meting the coding standards, however it is my intention to meet the coding standards.

Lastly apologies for changing the tags in my last post, no idea how I managed that.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: -Accessibility +Module review
  1. It is usually better to not write any copyright statement in the code; in the case you accept patches made from other users, the copyright statement would not be anymore valid. To make a comparison, in Drupal core code there is not any note about the code being copyrighted by Dries Buytaert; the code is copyrighted by him, and all the users who contributed to the code with patches.
    Stating that the code is copyrighted by an organization could create other issues; are you authorized to public on Drupal.org code that is copyrighted from your employer? As far as I recall, code developed by an employee during his work is owned by the employer.
  2. You cannot commit archives on Drupal.org, nor is it possible to remove archives committed on Drupal.org.
    I didn't check the content of the archive, but files that are available from third-party sites should not be committed in Drupal.org repository.
  3. Before to proceed with this CVS application, I would like to know what the maintainer of epub.module thinks about adding the code you developed to his module. Did you open a feature request for the existing project to add the features you implemented as separated module?

There is no reason to apologize for changing the issue tags; you could not know they are used from CVS maintainers when reviewing CVS applications.

verbosity’s picture

verbosity commented

1. as stated above the copyright text will be removed, I have checked and this is not an issue with my employer.

2. the contents of the archive are simply a couple of txt documents needed ( has no third party content), they are there to simplify the epub document creation process ( epub documents use a zip wrapper). However I shall remove these and implement their creation in code.

3. no, I will be happy to do so. I did contact the maintainer of the epub module, but have not yet got a reply.

verbosity’s picture

verbosity commented
Status: Needs work » Postponed

Ok as a follow up ( been thinking over this for the last few hours ).

I'm going to close my application for the moment, but open it with a smaller modified module for the purpose of producing DAISY documents, thus not duplicating the work done by others ( from what I've seen of the curent epub module this wouldn't be the easiest task and wouldn't have much crossover).

verbosity’s picture

verbosity commented

To reduce confusion between the module I've created and the extant one I'm going to call my module epublisher from here on in.

reopening my application on the following basis:

1. Issues in the current epub module are not being dealt with which my epublisher module does not have.
2. extended functionality, epublisher will attempt to turn any node into a document when requested.
3. CSS and image support for valid documents.
4. the ability to add audio to DAISY when available.

the epublisher module includes checkboxs for all content types, to enable the content types to have the option to be exported, and dependant on this each piece of content has a checkbox to allow its export to EPUB /DAISY, in addition epublisher has been given additional controls over how, and when an EPUB/DAISY Document is created:
on download -
on create/edit -
on cron -

I expect to have a more fully functional module for review in the next few days, though I do not expect this to pass the review stage.

In addition to that I'm working on a couple of modules to incorporate TTS ( Text to speech ) systems into drupal, the first, a rather small module to allow integration with yakitome should also be ready for review shortly.

baaj’s picture

baaj commented

Is your module released somewhere? I can't find it anywhere.
I'd love to give it a try. Thanks!

verbosity’s picture

verbosity commented
Status: Postponed » Active
StatusFileSize
new Freetts.zip11.29 KB

Hi,
@baaj the epublisher module is not currently released, I hope to have it released shortly.

In regards to my own post in #22 I decided against working with yakitome for various reasons. I have continued to work on TTS in order to be able to create fully fledged DAISY documents.

To that end I've developed the attached module Freetts.
The Freetts module provides text to speech functionality by taking the body text of a node and converting it into an .mp3 file via use of Java FreeTTS scripts running either locally or on a remote machine. LAME is also needed on the same machine as the FreeTTS system to facilitate conversion from .wav format to .mp3. MBrola support is included to allow additional voices and to support additional languages.
The creation of the audio file can be triggered via a provided link on the node page or via an option on the node add/edit form.

verbosity’s picture

verbosity commented
StatusFileSize
new Freetts0.7.zip11.24 KB
verbosity’s picture

verbosity commented
Status: Active » Needs review
StatusFileSize
new Freetts0.7.5.zip10.97 KB

improved version :
Includes a couple of minor fixes, some additional security checks and error reporting.

verbosity’s picture

verbosity commented
StatusFileSize
new freetts0.8.zip11.06 KB

Improved version:
I believe this version is a complete and final version.

verbosity’s picture

verbosity commented
StatusFileSize
new freetts0.85.zip11.18 KB

Updated version.
added error handling and admin checking

verbosity’s picture

verbosity commented

Realised I hadn't written a proper blurb for the freetts module here it is.

Freetts is a an interface to allow communication with either a remote or local installation of the Java based FreeTTS system.

Who is this module for
Drupal sites that want to provide the text content of nodes in a downloadable audio format, primarily as an mp3

How does it work
When a node is created / edited there is an option to create / re-create an audio file. Additionally there is a link beneath each node to allow this creation.
Once the creation of audio content is triggered, the system formats the data and sends it off to the system where FreeTTS creates a wav file and lame converts it into mp3. the freetts module downloads it and makes it available as a download on the node page.

Installation
install FreeTTS and LAME on the choosen machine.
copy the remote php files to a web available directory.
Optionally install MBrola and a MBrola voice ( allows for higher quality/ alternative or non-english voices) to the FreeTTS machine.
install and enable the FreeTTS module as you would anyother Drupal module
Configure the freetts admin settings

Other Similar modules:
Dixerit embeds a web-based TTS service and streams the audio over the web
Differencies: freetts downloads the audio allowing for podcasting abilities, the ability to create albums etc.

Motivation:
I had a need to create audio files easily in order to create full DAISY files for the epublisher module.

Further development possibilities:
a further developed api for FreeTTS LAME and MBrola
integration of similar TTs systems ( Festival )
Greater File management capabilities and options

brianV’s picture

brianV commented
Status: Needs review » Needs work

Some further notes:

  1. Please remove the __MACOSX directory and .DS_Store file from the archive.
  2. Please make sure that the proper CVS $Id$ tags are added to all files as per http://drupal.org/handbook/cvs/faq. This requirement will be going away once we move from CVS to Git next year. Specifically, README.txt, monster.php, freetts_remote.php and freetts.module needs CVS Id tags.
  3. Your hook_uninstall() should remove all variables created by your module (ie., freetts_remote_url, freetts_user_key etc.)
  4. Please remove the version info from the top of the .module file, unless you plan to update it every time you update your CVS repository....
  5. 'ARRAY(1)' should be lowercased here. Also, 'Freetts_page' should be 'freetts_page':
      $items['freetts/%'] = array(
    'title'           => 'Processing Audio',
    'access arguments' => array('freetts_Interface convert to audio'),
    'page callback'   => 'Freetts_page',
    'page arguments'  => ARRAY(1),
    'type'            => MENU_CALLBACK,
  );
  6. Please review the Coding Standards rules on whitespace usage. You have a lot of blank lines in your module, and are also missing spaces around operators in many cases. (ie., $test_row['ttsid']!='' should be $test_row['ttsid'] != ''
  7. Please make sure that your code commenting complies with the Drupal Doxygen guidelines as shown at http://drupal.org/node/1354 . This ensures that the API module and external sites like drupalcontrib.com can properly parse your code's documentation.
  8. Nothing wrong with using curl here ;). Just document that it's required for your module.
      // according to drupal spec the following should be used instead of curl
  // I'm having difficulty making this work
/*
   $request = drupal_http_request(
    $url,
    array('Content-Type' => 'application/x-www-form-urlencoded'),
    'POST',
    $fields_string);
    $result=$request->data;
    print_r( $result);
*/
  //do curl
  9. Server location should probably be configured in monster.php, since most people don't own that particular linode ;).
    //return url
//-------------------------------------------------------
$return_link='http://li265-211.members.linode.com/'.$basename.'.mp3';
  10. Should monster.php be renamed something more descriptive to the users, such as freetts_config.php?
  11. Please run your module through the Coder module code review. This will pick up a lot of coding standards, function misuse, security, and internationalization issues I may have missed. See http://drupal.org/project/coder
verbosity’s picture

verbosity commented

hi,
Firstly thank for reviewing my module and thanks for the feedback, quite a few things I missed eh?

1,2,3,6,7 noted I'll make the changes

4. noted and I'm happy to remove it, however Id rather keep it in place for now ( until it gets accepted) it helps me keep track in my local development environment , though if this is a block then I'll remove it.

8. Thanks, I'll remove the commented code,

9. good catch, silly of me to have missed that.

10. no reason there shouldn't be a rename. personally I call stuff by non-standardish names, that way its harder to find them, for the same reason user_key and user_ssid are called what they are. freetts_config.php is pretty good though

11. for the record I did pass my module through coder, several times, its a fairly standard part of my testing cycle now. In saying that a couple of the files weren't checked

brianV’s picture

brianV commented

re #4: This isn't a blocker - that was more of a 'best practice' suggestion.

re #10: I wouldn't call this a blocker necessarily either, provided it si very clearly documented. However, it's not the most usable either, and tough for a user to search for and find if they don't have the README.txt immediately available either.

verbosity’s picture

verbosity commented
StatusFileSize
new freetts0.86.zip10.1 KB

ok I've made all the changes.

I admit I've still got a bit of work on the documentation even in this version, but at least the one real new function is documented.

Edit: I hadn't noticed brianV's post above, however I've made the changes that had been pointed out in points 4. and 10.

brianV’s picture

brianV commented

Looks much better! A few more notes:

  1. Please review your inline comments, and ensure that they all start with a Capital, and end with a period.
  2. Please remove commented out debug code.
    //echo "lame debugg <br/>";
//print_r($output);
  3. The __MAXOSX and .DS_Store hidden files / directories are still in the archive. Not a blocker, just make sure they aren't committed to CVS.
  4. monster.php is still named that, although the require_once() in freetts_remote.php has been updated to call freetts_config.php.
  5. Please make sure freetts_remote.php complies with the Drupal coding standards as well. Even though it isn't run as part of a Drupal site, we like to see everything using the same coding standards to make it easy for contributors.
verbosity’s picture

verbosity commented
Status: Needs work » Needs review
StatusFileSize
new freetts0.9.zip7.01 KB

again all changes made, I hope this counts as good to go :)

brianV’s picture

brianV commented
Status: Needs review » Reviewed & tested by the community

I think this looks great now.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Reviewed & tested by the community » Fixed

Welcome on board!

Status: Fixed » Closed (fixed)
Issue tags: -Module review

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +Module review

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Component: Miscellaneous » new project application
Assigned: Unassigned » avpaderno
Issue summary: View changes