Skip to content

Conversation

deziev
Copy link
Contributor

@deziev deziev commented Oct 2, 2019

This PR contains refactoring of file system related stuff. New features will simplify fs management.
Also adds new error handling method and command Error class - watch terminal.js runCommand method;

#1
#11

deziev added 3 commits October 2, 2019 02:52
Remade 'cd' command with usage of
new FileSystem class
Add 'la' flag support ot 'ls' command
Add path resolver which creates absolute
path from current location and relative path,
passed as params

syntaxseed#11
@deziev
Copy link
Contributor Author

deziev commented Oct 2, 2019

To check how new commands work just rebuild bundle or uncomment scripts in index.html. Note that <script src="js/filesystem.js"></script> should be first in scripts

@syntaxseed
Copy link
Owner

syntaxseed commented Oct 2, 2019

So, the idea behind the Filesystem's initial state being defined as it was, is that anyone using this application can easily customize their own intial state of it.

Can this be tweaked to read the initial state from a json file?

Other than that, this looks cool!

@syntaxseed
Copy link
Owner

Other than wanting the initial state in a json file, this is really great. Going to merge this in before other PRs so we can all use this format.

@deziev
Copy link
Contributor Author

deziev commented Oct 2, 2019

Also ls working as it intended

custom-ls-usage

@deziev
Copy link
Contributor Author

deziev commented Oct 2, 2019

Can this be tweaked to read the initial state from a json file?

Yeah, that's easily could be done by adding toPlain/restore methods in FileSystem class. Can you create new issue for fs state restoring

@syntaxseed
Copy link
Owner

syntaxseed commented Oct 2, 2019

Pulled this down and had a look.

  • I really like how this is working and the new ls listing is great!

A few concerns:

  • It breaks the touch command which no longer works.
  • It introduces several new functions in the global namespace. Since this project can be embedded into other projects or websites I'm trying to keep the global namespace as clean as possible. Can some of the smaller functions and constants be moved into, a containing class? Like the Filesystem class for example.
  • system.js should contain only the definitions of the built in commands. Can the functions placed in here be moved as per the comment above? Ie maybe contained in the Filesystem class? Or those specific to the ls command, into the definition of that command itself.

@deziev
Copy link
Contributor Author

deziev commented Oct 2, 2019

It breaks the touch command which no longer works.

Honestly, I did not tested other commands :D

It introduces several new functions...

Im not familiar with frontend development, so had no idea bout that. Moreover that browser module system bugs me out. That make hard to write clean, readable code

system.js

Will fix that

@deziev
Copy link
Contributor Author

deziev commented Oct 3, 2019

@syntaxseed I think I finished with improvements and your suggestions on this PR

@syntaxseed
Copy link
Owner

syntaxseed commented Oct 3, 2019

Awesome...

A small note before I pull this down...

Filename allowed chars should be: [^A-Za-z\d\.\-_~]

Directory name allowed chars: [^A-Za-z\d\-_~]

(If you support 'hidden' directories, you can allow dots in directory names too!)

I saw this in your changes:

if (!newFileName.match(/[A-z]+/)) {
    throw new CmdValidationError('touch', `${path}: Invalid file name.`);
}

PS - This project originated as a way for me to practise my Javascript skills... so appologies that it will take me some time to go over and fully grok your PR. But I'm very excited about your refactor I think it's a big improvement. :)

@syntaxseed syntaxseed added next to be merged This PR will likely be merged next, so others may need to rebase after that. review in progress A review of this item is underway. labels Oct 3, 2019
@syntaxseed syntaxseed mentioned this pull request Oct 3, 2019
@syntaxseed
Copy link
Owner

@deziev ... does your refactor allow 'hidden' directories? If so, we can allow dots in directory names too. :)

@deziev
Copy link
Contributor Author

deziev commented Oct 3, 2019

@deziev ... does your refactor allow 'hidden' directories? If so, we can allow dots in directory names too. :)

oh, missed that one) yes, new ls command work with hidden units. mb fix that in Path refactoring?

For me this project is great chance to better understand how linux terminal works, so im interested in further development. also past few years I was working with nodejs so feel free to ask for help)

@syntaxseed
Copy link
Owner

syntaxseed commented Oct 3, 2019

Looked this over and it looks great. I made some tiny changes... but my minifier isn't working meaning something in your PR isn't ES 2015 compatible.

Also the touch command only works from the root now.

@deziev
Copy link
Contributor Author

deziev commented Oct 3, 2019

something in your PR isn't ES 2015 compatible.

oh, everything in my PR in es6, arrow functions, literals, let - const, Set - Map etc. Its not a good idea to use es5 in 2019))

@godcrampy
Copy link
Contributor

@deziev true I think it'll be a good idea to refactor this repo to ES6. Also @syntaxseed can we migrate to Node as it'll allow us to leverage tools like ES6 and Webpack to automate the minifying process and making the code cleaner and more obvious to read.

@syntaxseed
Copy link
Owner

syntaxseed commented Oct 8, 2019

Ok. Let's talk about this.

I don't have any experience with Node... and the original idea was to create something that could be easily just dropped into another website. If we go the ES6 route, I will need to transpile and minify a bundle that can be provided for people who don't want to bother with that. We can keep the custom commands (commands.js) and the initial filesystem state (filesystem.js) in a ES5 ready to use state, and we can write all the internal stuff in ES6 and then transpile it to a minified bundle for deployment.

Can anyone point me to the easiest way to get started with this for a Linux dev environment? I'm a PHP dev so I don't have npm or any JS transpilers installed. But I agree that this sounds like the best route. Willing to learn how it works!

Is this something I can accomplish with Babel alone? Or do I need to install npm and Node?

@syntaxseed
Copy link
Owner

syntaxseed commented Oct 8, 2019

I noticed I already have Node/NPM installed. Upgraded both to the latest:

node -v
v12.11.1

npm -v
6.11.3

I am in the process of going through a Babel CLI tutorial.

syntaxseed added a commit that referenced this pull request Oct 8, 2019
syntaxseed added a commit that referenced this pull request Oct 8, 2019
@syntaxseed syntaxseed merged commit 872d152 into syntaxseed:master Oct 8, 2019
@syntaxseed syntaxseed removed the next to be merged This PR will likely be merged next, so others may need to rebase after that. label Oct 8, 2019
@syntaxseed
Copy link
Owner

This PR has been merged. But it has introduced a few issues:

  • Changes to the filesystem no longer are saved to LocalStorage and retrieved properly.
  • Initial Filesystem definition is not in a format that can be easily customized. Need to read initial state from a Json file or similar.
  • Minified bundle is no longer accurate. Need to configure a Babel process to transpile to ES5 and minify.

I will create separate issues for these.

Thanks for your contribution @deziev!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review in progress A review of this item is underway.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants