Luci-app-filemanager #7300

rdmitry0911 · 2024-10-02T11:37:52Z

rdmitry0911 · 2024-10-15T17:12:58Z

New version includes:

Navigation with resizable window/columns and with list sorting
Directory and files creation/renaming/removing
Directory and files attributes changing
Large files uploading/downloading
Large files editing
Storage of Interface settings in config file

systemcrash · 2024-10-15T18:33:08Z

Looking good. Avoid merge commits. Just rebase your work on master.

rdmitry0911 · 2024-10-16T04:20:59Z

Looking good. Avoid merge commits. Just rebase your work on master.

This way is better?

dannil

Please follow the contribution guidelines; squash both your commits together and name it something like luci-app-filemanager: add app and put further verbose explanation into the commit body instead of the subject. And just do a rebase, right now you've included every commit again which (I guess) was newer than your HEAD.

systemcrash · 2024-10-19T16:45:06Z

Amend your commit subject

rdmitry0911 · 2024-10-19T19:58:10Z

Amend your commit subject

What would you propose?

systemcrash · 2024-10-23T20:30:25Z

OK great. I think this is ready. @dannil @stokito @feckert any opinions?

stokito · 2024-10-23T20:59:00Z

It solves user problems, won't affect existing functionality so let's merge. There are some styling issues, but that can be improved gradually latter. This is a first time contribution for @rdmitry0911 so I don't want to overwhelm him with minor comments but rather send a PR myself.
Users need for some solution today and this can help a lot for them.

systemcrash · 2024-10-23T21:02:53Z

Do you want to take it for a test-drive @stokito ?

stokito · 2024-10-23T21:49:37Z

I tested and it works. My main comments are:
0. Not sure if we need options for styling like width/height, padding etc.

Rename Current Directory: to a Starting Folder:
Use Luci's own uci.js to get options.
Use Luci's own fs to get list of files instead of calling of ls

In general this may be confusing for users with existing plugin filebrowser. Maybe it worth to call it luci-app-filebrowser-extended or something like that. But instead I hope that one day we can merge the luci-app-filebrowser into Luci itself once it become more mature.

systemcrash · 2024-10-23T22:13:44Z

@rdmitry0911 please address @stokito's comments.

stokito · 2024-10-23T22:33:15Z

They aren't critical and we may work out them later. The app works and can be merged

rdmitry0911 · 2024-10-24T09:05:58Z

I tested and it works. My main comments are: 0. Not sure if we need options for styling like width/height, padding etc.

Initially there were no settings for width/height and padding, but finally I added them as different users have different environments and it is convenient to start this app with width/height, padding you like rather then adjust them right next to start.

1. Rename `Current Directory:` to a `Starting Folder:`

No problem, I'll do that. Should I change a version number just for this commit or it is better to collect several changes and commit them together for version changing?

2. Use Luci's own uci.js to get options.

3. Use Luci's own fs to get list of files instead of calling of `ls`

In fact initially it was done that way, however, using ls, cat and the likes makes the code more compact. And as far as those utils are required for running any system, using them is harmless.

systemcrash · 2024-10-24T16:01:11Z

I would prefer to use the built-in fs lib than suffering monstrous regexp to process ls output. fs has a bunch of stable and usable properties. Then you avoid risks associated with parsing of dates, times and permissions.

rdmitry0911 · 2024-10-25T09:53:36Z

I would prefer to use the built-in fs lib than suffering monstrous regexp to process ls output. fs has a bunch of stable and usable properties. Then you avoid risks associated with parsing of dates, times and permissions.

The problem of using 'fs.list()' is that I can't find a way to get information about symbolic links. They are returned as directories. I have to use 'fs.stat()' for every directory in a list to find links. It produces significant overhead in comparison with 'ls'

stokito · 2024-10-25T14:02:44Z

That's true, the /var/ is a symlink but is returned as:

          {
            "type": "directory",
            "inode": 1,
            "ctime": 1729862220,
            "atime": 1729222217,
            "uid": 0,
            "mtime": 1729862220,
            "gid": 0,
            "mode": 17407,
            "name": "var",
            "size": 820
          },

The /bin/ash is a file symlink but this also not represnted:

          {
            "type": "file",
            "inode": 35494,
            "ctime": 1708105277,
            "atime": 1729222218,
            "uid": 0,
            "mtime": 1708088432,
            "gid": 0,
            "mode": 33261,
            "name": "ash",
            "size": 524309
          },

We'll need to add fix this in the RPCD itself.

The fs.js says that the symlink will be in the type:

 * @typedef {Object} FileStatEntry
 * @property {string} type - Type of the entry, one of `block`, `char`, `directory`, `fifo`, `symlink`, `file`, `socket` or `unknown`

We better not to change the type but instead introduce a separate property for symlinks. Maybe we need to support the hardlinks too, not sure.
@systemcrash maybe you can create a feature request for this?

rdmitry0911 · 2024-10-25T15:18:39Z

We better not to change the type but instead introduce a separate property for symlinks. Maybe we need to support the hardlinks too, not sure.

I would also return a target of a symlink in a separated property to avoid extra fs.stat()

stokito · 2024-10-25T16:08:13Z

Makes sense. Also eventually we'll need to add a button to create a symlink.
@rdmitry0911 please let us know if you'll add anything to the pr or it can be merged. I hope everybody here are fine to merge the app. Once it merged it may attract more contributors

rdmitry0911 · 2024-10-25T16:32:45Z

Makes sense. Also eventually we'll need to add a button to create a symlink. @rdmitry0911 please let us know if you'll add anything to the pr or it can be merged. I hope everybody here are fine to merge the app. Once it merged it may attract more contributors

I think you can merge it.

systemcrash · 2024-10-25T19:31:33Z

Try lstat? https://ucode.mein.io/module-fs.html#lstat

stokito · 2024-10-25T19:42:12Z

we better to fix the rpcd in the first place than use a workaround with ucode

systemcrash · 2024-10-25T20:02:57Z

Ah yeah - it doesn't exist in the lib yet. Worth it to add it.

systemcrash · 2024-10-25T20:16:31Z

@jow- in da house. Can we get a linkstat lstat function in rpcd please?

rdmitry0911 · 2024-10-29T08:50:20Z

Is there anything I can do to accelerate approvals for this PR?

systemcrash · 2024-10-29T17:26:55Z

OK, I have a suitable patch for rpcd, which may take a while to get added. But here's how it works:

root@OpenWrt:/# ubus call file lstat '{"path":"/etc/os-release"}'
{
	"path": "/etc/os-release",
	"type": "symlink",
	"size": 21,
	"mode": 41471,
	"atime": 1727094886,
	"mtime": 1727094886,
	"ctime": 1727094886,
	"inode": 219,
	"uid": 0,
	"gid": 0
}
root@OpenWrt:/# ubus call file stat '{"path":"/etc/os-release"}'
{
	"path": "/etc/os-release",
	"type": "file",
	"size": 528,
	"mode": 33188,
	"atime": 1727094886,
	"mtime": 1727094886,
	"ctime": 1727094886,
	"inode": 1124,
	"uid": 0,
	"gid": 0
}
root@OpenWrt:/# ubus call file stat '{"path":"/var"}'
{
	"path": "/var",
	"type": "directory",
	"size": 520,
	"mode": 17407,
	"atime": 1730221560,
	"mtime": 1730221753,
	"ctime": 1730221753,
	"inode": 1,
	"uid": 0,
	"gid": 0
}
root@OpenWrt:/# ubus call file lstat '{"path":"/var"}'
{
	"path": "/var",
	"type": "symlink",
	"size": 3,
	"mode": 41471,
	"atime": 1727094886,
	"mtime": 1727094886,
	"ctime": 1727094886,
	"inode": 1264,
	"uid": 0,
	"gid": 0
}
root@OpenWrt:/# ubus call file stat '{"path":"/bin/ash"}'
{
	"path": "/bin/ash",
	"type": "file",
	"size": 405522,
	"mode": 33261,
	"atime": 1727094886,
	"mtime": 1727094886,
	"ctime": 1727094886,
	"inode": 15,
	"uid": 0,
	"gid": 0
}
root@OpenWrt:/# ubus call file lstat '{"path":"/bin/ash"}'
{
	"path": "/bin/ash",
	"type": "symlink",
	"size": 7,
	"mode": 41471,
	"atime": 1727094886,
	"mtime": 1727094886,
	"ctime": 1727094886,
	"inode": 13,
	"uid": 0,
	"gid": 0
}

stokito · 2024-10-29T17:42:13Z

We may merge it as is and the rpcd chamge can be made later

rdmitry0911 · 2024-10-29T20:12:48Z

OK, I have a suitable patch for rpcd, which may take a while to get added. But here's how it works:

This patch resolves only lack of individual file information. Another patch for fs.list() is required to get all the information about folder contents at once like using ls -l.

rdmitry0911 · 2024-10-29T20:13:43Z

We may merge it as is and the rpcd chamge can be made later

Agree. The app just works. We can improve it later on

applications/luci-app-filemanager/Makefile

applications/luci-app-filemanager/root/usr/share/rpcd/acl.d/luci-app-filemanager.json

applications/luci-app-filemanager/htdocs/luci-static/resources/view/system/filemanager.js

applications/luci-app-filemanager/po/ar/filemanager.po

- Navigation in resizable window - Scrollable and sortable list of files with name, type, creation date/time columns - Directory and files creation/renaming/removing - Directory and files attributes changing - Large files uploading/downloading - Large files editing - Storage of interface settings in config file Signed-off-by: Dmitry R <[email protected]> luci-app-filemanager: Code improvements and translations luci-app-filemanager: добавлены новые изменения

rdmitry0911 · 2024-11-12T15:14:51Z

Is there anything else I can do to accelerate approvals for this PR?

rdmitry0911 force-pushed the luci-app-filemanager branch from 45f713c to b2263ae Compare October 16, 2024 04:19

dannil suggested changes Oct 17, 2024

View reviewed changes

rdmitry0911 force-pushed the luci-app-filemanager branch from b2263ae to e5afb22 Compare October 19, 2024 15:58

rdmitry0911 force-pushed the luci-app-filemanager branch from e5afb22 to 32cf670 Compare October 20, 2024 13:35

stokito mentioned this pull request Oct 25, 2024

Add File Manager / File Browser based on form.FileUpload #6553

Merged

systemcrash mentioned this pull request Oct 29, 2024

file: linkstat to get link stat info openwrt/rpcd#9

Open