Electronics & Programming

develissimo

Open Source electronics development and programming

  • You are not logged in.
  • Root
  • » KiCAD
  • » [Kicad-developers] Using FILE_LINE_READER in pcbnew [RSS Feed]

#1 Jan. 12, 2011 02:36:36

Dick H.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Kicad-developers] Using FILE_LINE_READER in pcbnew


On 01/11/2011 04:07 PM, Marco Mattila wrote:
> Hi,
>
> When discussing the possibility to save plotting options into board
> files earlier, Dick suggested that it might be a good idea to start
> using FILE_LINE_READER in pcbnew. I made a patch that basically
> changes GetLine to use a LINE_READER instead of a file directly. The
> old function was renamed to GetLineD, since it is still currently used
> somewhere else outside pcbnew in its old form. Changing the rest of
> the code to use the new GetLine seems to be fairly trivial in most
> places:
>
> Old code:
>
> char line
> GetLine( file, line, ...)
> ...
>
> New code:
>
> char* line;
> GetLine( &fileLineReader )
> line = (char*) fileLineReader;
> ...
>
> Because it's a rather large patch, I thought that I should post it
> here for comments before committing. If my approach seems reasonable,
> I can clean up and amend the patch so that the new GetLine is used
> everywhere (including eeschema) and GetLineD is not needed anymore.


Marco,

I'm glad you asked.

There is good and bad in the patch, but enough bad that I think we need to
keep talking.

Let me get back to you within the next couple of hours please, but do not
commit.

Thanks,

Dick



_______________________________________________
Mailing list:https://launchpad.net/~kicad-developersPost to : kicad-developers@lists.launchpad.net
Unsubscribe :https://launchpad.net/~kicad-developersMore help :https://help.launchpad.net/ListHelp

Offline

#2 Jan. 12, 2011 04:53:05

Dick H.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Kicad-developers] Using FILE_LINE_READER in pcbnew


Marco,


*))))))))) First the good:

1) The idea of passing a LINE_READER* to the major functions is correct
I think. For example, this is good:

-bool DIMENSION::ReadDimensionDescr( FILE* File, int* LineNum )
+bool DIMENSION::ReadDimensionDescr( LINE_READER* aReader )

2) Not worrying about the trailing /r/n or /n or /r at the end of line is good.
I think and would hope that client code is not going to be too affected
because you removed the strtok() call. /n and /r/n or /r is all just
whitespace,
no different than a tab or space, at least to me or code I would write.
But it depends on client code, I assume you have some feel for how stupid
the client code might be in this regard. Your risk is low here and the
strtok() being removed is reasonable.


*)))))))) Then the bad:

I don't like the GetLineD() introduction. I would say just overload
GetLine() since
it returns the same type. Code can use either, with *same* function name.
This way existing code is not affected by this change in direction.

I am not that fond of GetLine() in any case, and surely would not want to
introduce
it into places that it is not already being used. All it does now is strip out
blank lines and comments. For an alternative, made possible by a commit I
just made
this evening to base class LINE_READER, take a look at class
new/filter_reader.cpp.

If you like this, move it up into /include and /common library. Otherwise,
it can just stay in /new as an example or for future reference.

filter_reader.cpp is *also* attached for reference.

Thank you for your efforts!

Dick#include <richio.h>
#include <string.h>


/**
* Class FILTER_READER
* reads lines of text from another LINE_READER, but only returns non-comment
* lines and non-blank lines from its ReadLine() function.
*/
class FILTER_READER : public LINE_READER
{
LINE_READER& reader;

public:

/**
* Constructor ( LINE_READER& )
* does not take ownership over @a aReader, so will not destroy it.
*/
FILTER_READER( LINE_READER& aReader ) :
reader( aReader )
{
}

unsigned ReadLine() throw( IO_ERROR )
{
unsigned ret;

while( ( ret = reader.ReadLine() ) != 0 )
{
if( !strchr( "#\n\r", reader ) )
break;
}
return ret;
}

const wxString& GetSource() const
{
return reader.GetSource();
}

char* Line() const
{
return reader.Line();
}

unsigned LineNumber() const
{
return reader.LineNumber();
}

unsigned Length() const
{
return reader.Length();
}
};_______________________________________________
Mailing list:https://launchpad.net/~kicad-developersPost to : kicad-developers@lists.launchpad.net
Unsubscribe :https://launchpad.net/~kicad-developersMore help :https://help.launchpad.net/ListHelp

Offline

#3 Jan. 13, 2011 07:22:38

Dick H.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Kicad-developers] Using FILE_LINE_READER in pcbnew


Marco Mattila,

You get the highest praise I can offer, and that is that I could not have
written it any better myself.

Truly outstanding.


+ while( aReader->ReadLine() )
{
+ Line = aReader->Line();


I see you even caught the fact that if there is an absurdly long line, >
5000, that the Line pointer can change as the LINE_READER reallocates and
moves the internal buffer. I see that you only pay for the minor overhead
of traveling through the virtual functions once per line.

Everything is just plain excellently done.


> I guess that the strtok was there to take care of situations where a
> file saved in windows is opened in linux (?). Opening a board having
> \r\n line endings seems to work using FILE_LINE_READER. A file using
> \r's only would not work.

I think fgets() will hide the platform specific ugliness well enough. No
sleep lost here.

The requirement for XML parsers would make a good requirement for how
fgets() should work:http://www.w3.org/TR/REC-xml/#sec-line-endsQuoted here:

To simplify the tasks of applications
<http://www.w3.org/TR/REC-xml/#dt-app>, the XML processor
<http://www.w3.org/TR/REC-xml/#dt-xml-proc> /MUST/ behave as if it
normalized all line breaks in external parsed entities (including the
document entity) on input, before parsing, by translating both the
two-character sequence #xD #xA and any #xD that is not followed by #xA to a
single #xA character.


fgets() may not do this on all our 'supported' platforms. If what comes in
are lines of text, and \r and \n are whitespace, same as \t and ' ', then
reasonably written line using code should be fine.

(Supported platforms is an oxymoron when you think about it. Isn't a
platform supposed to support us?) If we find one that does not, or we get
grief, then we can re-implement FILE_LINE_READER::ReadLine(), whose API can
be protected contractually, because it is worth protecting. At that point a
looping fgetc() call comes to mind, under the hood of ReadLine().


Thanks for your work. I literally could not have done it better myself.

Dick




_______________________________________________
Mailing list:https://launchpad.net/~kicad-developersPost to : kicad-developers@lists.launchpad.net
Unsubscribe :https://launchpad.net/~kicad-developersMore help :https://help.launchpad.net/ListHelp

Offline

  • Root
  • » KiCAD
  • » [Kicad-developers] Using FILE_LINE_READER in pcbnew [RSS Feed]

Board footer

Moderator control

Enjoy the 12th of December
PoweredBy

The Forums are managed by develissimo stuff members, if you find any issues or misplaced content please help us to fix it. Thank you! Tell us via Contact Options
Leave a Message
Welcome to Develissimo Live Support