sugarlabs / Pippy

Pippy allows the student to examine, execute, and modify simple Python programs. In addition it is possible to write Python statements to play sounds, calculate expressions, or make simple text based interactive games.
GNU General Public License v2.0
11 stars 35 forks source link

Prevent inadvertently missing files in distutils export #80

Closed tchx84 closed 4 years ago

tchx84 commented 4 years ago

New source files are added with spaces and without the dot py extension, and therefore are ignored when exporting packages. Better to use a valid name in the first place.

quozl commented 4 years ago

Thanks. Tested. Can the problem be fixed by not ignoring files when exporting packages?

tchx84 commented 4 years ago

Thanks. Tested. Can the problem be fixed by not ignoring files when exporting packages?

You mean doing both things? Because I think we should still use valid names for new sources.

quozl commented 4 years ago

I see. What's the criteria for valid names? Is there a validator? At the moment, this patch only changes the default name for new files. Files can be renamed to an invalid name.

tchx84 commented 4 years ago

No spaces and the proper extension is a good start. I am open to suggestions, that don't require the user to guess how files should be named or that changes file names without the user knowing about the change..

tchx84 commented 4 years ago

I found something interesting here https://github.com/sugarlabs/Pippy/blob/37b6d22c03c55badd482207f18afcd9b6769524d/notebook.py#L325

Maybe there's a way we can use it here, with the UX restrictions I mentioned.

quozl commented 4 years ago

Yes, I'm familiar with that, as I had a lot of work to fix things for collaboration. https://github.com/sugarlabs/Pippy/commit/178abe174ee1b84a832bd694b8364cc1e7b90b89.

Changing naming again may also break collaboration between old and new versions.

I'm also worried about any lesson plans.

If it is only distutils that needs the names valid, can't we rename just before running distutils in export? Or show a message asking for names to be changed?

By the way, many of the example files are untyped too.

tchx84 commented 4 years ago

Yes, I'm familiar with that, as I had a lot of work to fix things for collaboration. 178abe1.

Changing naming again may also break collaboration between old and new versions.

That's a valid concern.

I'm also worried about any lesson plans.

If it is only distutils that needs the names valid, can't we rename just before running distutils in export? Or show a message asking for names to be changed?

We could, I am just not sure would be a good UX since the rename happens without user knowing (e.g. later they wouldn't be able to import with the same they knew).

Maybe rename on export and then show a notification if one of these files got renamed ?

By the way, many of the example files are untyped too.

quozl commented 4 years ago

Maybe just refuse export with an Alert if the names aren't valid? It's not like exporting is common.

tchx84 commented 4 years ago

@quozl that should do! At least it's giving the user a suggestion.

quozl commented 4 years ago

Thanks!