-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementing UNIX-like secrets ls
command
#255
Conversation
This is the current state of the For now, we need to remove permissions and users columns and fill it with something else. That is something we need to discuss. I have been thinking that we could do something like printing the vault ID instead of the permissions, and printing the type of file (directory or file) in place of the user and operators. |
The metadata of So in a way we can cut down the number of irrelevant information while still preserving some interesting data. Like But other ideas is possible. |
|
The key thing here is that LS doesn't really need to match the Unix LS command. It's not that well defined anyway. |
And more advanced features to LS can be added in later. I want to see the other commands implemented as they are more important. |
I have noticed that Moreover, by default, the 'glob' select (which is used internally to mean 'select everything') actually excludes hidden files, which is different to the behaviour in shell. This is what I have to current do to properly fetch all results, including hidden files, by default. function formatPattern(pattern: string, directory: boolean = false): string {
function formatPattern(pattern: string, directory: boolean = false): string {
if (pattern == '/') return '{*,.*}';
if (pattern.charAt(0) == '/') pattern = pattern.substring(1);
if (directory) pattern = `${pattern}/{*,.*}`;
else pattern = `{*${pattern},.*${pattern}}`;
return pattern;
} The NPM page on I'm not sure if this is the best way to be handling this, so I would like @tegefaulkes to provide me feedback on this. |
I am nearing completion on both the PRs in PK and PKCLI. The only major task that I have to do in MatrixAI/Polykey#785 is adding the second RPC handler for streaming file contents over |
If two options coincide with their short name, then the option which was registered first would be set. For example, if I have two options: If the [aryanj@matrix-34xx]$ pk test flags
Options:
-l --long Use a long listing format (default: false)
-l --longer Use a longer format (default: false)
[aryanj@matrix-34xx]$ pk test flags -l
long: true
longer: false
[aryanj@matrix-34xx]$ pk test flags -l -l
long: true
longer: false
[aryanj@matrix-34xx]$ pk test flags -l --longer
long: true
longer: true If both the short and long name are the same, then the second registered option will always be [aryanj@matrix-34xx]$ pk test flags -l
long: true
another long: undefined |
Remember the discussion. We're not using the I think the we should at least stick to the format Lastly, when checking the conflicting short options. Remember that we have multiple level commands such as |
I thought that we weren't going to use the 'glob' feature of the
I also agree here, but it is true that most of that information is useless for us. We might be able to implement another flag which can use a newer formatting made for Polykey. The |
There are no options for the The results were as expected. As [aryanj@matrix-34xx]$ pk secrets ls
Options:
-np, --node-path <path> Path to Node State (default: "/home/aryanj/.local/share/polykey", env: PK_NODE_PATH)
-pf, --password-file <path> Path to Password
-f, --format <format> Output Format (choices: "human", "json", default: "human")
-v, --verbose Log Verbose Messages (default: 0)
-l --longer longer (default: false)
-ni, --node-id <id> (env: PK_NODE_ID)
-ch, --client-host <host> Client Host Address (env: PK_CLIENT_HOST)
-cp, --client-port <port> Client Port (env: PK_CLIENT_PORT)
-l --long Use a long listing format (default: false)
-h, --help display help for command
[aryanj@matrix-34xx]$ pk secrets ls -l
long: false
longer: true
...
[aryanj@matrix-34xx]$ pk secrets -l ls
long: false
longer: true
...
[aryanj@matrix-34xx]$ pk -l secrets ls
long: false
longer: true
... |
I can't tell if you applied the two |
The [aryanj@matrix-34xx]$ pk --longer secrets ls
works
[aryanj@matrix-34xx]$ pk --long secrets ls
fails
[aryanj@matrix-34xx]$ pk secrets --long ls
fails
[aryanj@matrix-34xx]$ pk secrets --longer ls
works
[aryanj@matrix-34xx]$ pk secrets ls --longer
works
[aryanj@matrix-34xx]$ pk secrets ls --long
works
[aryanj@matrix-34xx]$ pk secrets ls --long --longer
works |
It's not recommended for people to create scripts depending on ls. Like I said ls is not a standardized command. So regular ls should just do the simple thing which in our case is listing the contents one line at a time. As for -l option, yea it could work for us to show more data. It should still use one of the standard output formatters we use though. |
So does this mean that if there is a global option like -l, this would conflict with our local option -l? At any case you then want to avoid any short or long options that is likely to be in conflict with any global option that currently has been defined on PK. Again I would reduce the scope of this issue/pr to focus on the basic functionality first, not necessarily all the options of the Unix commands. It would take too long to complete otherwise. |
Yes, if a global option and a local option both have conflicts, then the global one will take precedence. I am working on the basic functionality, and that seems to be almost done. I can currently list the files in a vault, list files within a directory inside the vault, and list them in long format looking like this: [aryanj@matrix-34xx]$ pk secrets ls myvault:/ -l
FILE 65 Aug 19 10:07 .hidden-file.txt
FILE 175 Jun 13 16:53 another.txt
FILE 54 Jun 13 16:53 newsecret.txt
FILE 17 Jun 13 16:51 secret1.txt
DIR 0 Aug 19 12:21 test
[aryanj@matrix-34xx]$ pk secrets ls myvault:test -l
FILE 45822 Aug 19 12:21 test/.hidden-document.docx
FILE 34478 Aug 19 11:20 test/image.jpg
FILE 3364 Aug 19 11:19 test/something.txt |
BTW, GlobWalk just walks the tree based on a glob pattern. It isn't an implementation for LS. So if it only matches the directory it will only output the directory and not its contents. |
I prefer a suffix of |
I wonder would the hardlink count be useful. |
Now, the output of the long list looks like this: [aryanj@matrix-34xx]$ pk secrets list myvault:/ -l
65 Aug 19 10:07 .rippling-dns-record.txt
175 Jun 13 16:53 another.txt
54 Jun 13 16:53 newsecret.txt
17 Jun 13 16:51 secret1.txt
- Aug 19 12:21 test/
[aryanj@matrix-34xx]$ pk secrets list myvault:/ -lh
Size Date Modified Path
65 Aug 19 10:07 .rippling-dns-record.txt
175 Jun 13 16:53 another.txt
54 Jun 13 16:53 newsecret.txt
17 Jun 13 16:51 secret1.txt
- Aug 19 12:21 test/ |
Let's add back in the hardlink counts.
If this is true, then how is your I'm basically pointing out here, that if the global option takes precedence, you cannot have any conflicting local options. Global options stem from the |
BTW if you're doing prefix offsets, that means you are buffering the output in the case of the long option? |
Here is the ACTUAL posix spec of Have a read of the details there. The |
Those are the entry type characters. In EFS, I don't believe we have block and character files. But we do have symlinks. So...
Can be used. |
|
That does imply buffering up to sort by alphanum. We would just assume english. But technically unix paths are allowed any character except Would we then use LANG and other things for sorting? That doesn't even exist on Windows, so I guess we would just sort alphanum for now. Whatever JS does with https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort. |
So I would do something like this:
What do you think? Maybe even
It's just the lack of separation between hardlinks and the file size is a bit difficult to decipher. Without hardlinks...
Hardlink information - primarily indicates whether the file data is going to be deleted. It is useful information as it will even tell you how many files in the directory. |
Take note that the date time showing, changes dpeending on whether it is a regular time date vs year. I think also it is localized against the local timezone too. So by default the files contain the UTC timezone, you'd want to localise that time date too. |
Due to both alphabetical ordering AND prefix offsetting for formatting, that means the long format is the same as the table output formatter... However I believe they use space character, not tab character, so I guess it's not a table output formatter. Either both styles involve buffering. It's not a stream based listing command, unless we implement |
Oh and the |
In your tests, you should also test for possiblities of filenames as per https://stackoverflow.com/a/31976060. As well as dealing with potential newlines within a file name. And how the For example if a file name is Furthermore So single quotation would occur for some whitespace characters. However |
Most likely because
Yes, I am buffering the data by adding it to an array to apply further formatting to it. I haven't currently sorted the entries, just storing them in their iNode order and listing them in same.
Yes, this is how it is handled currently. However, there is one catch. If we want to list the contents of the root directory, we do: [aryanj@matrix-34xx]$ pk secrets ls newvault:
[wrong command]
[aryanj@matrix-34xx]$ pk secrets ls newvault:/
/.hidden-file.txt
/another.txt
/newsecret.txt
/secret1.txt
/test
[aryanj@matrix-34xx]$ pk secrets ls newvault:/test
/test/.document.docx
/test/image.jpg
/test/something.txt
[aryanj@matrix-34xx]$ pk secrets ls newvault:test
test/.document.docx
test/image.jpg
test/something.txt Basically, leading slashes changes the output of the listing, and I'm not sure if this needs to change, or not.
That is why I added an option to display the header in long formatting, to make it easier to understand. Maybe I can enable it by default?
By default, the date data is stored in milliseconds since 1 Jan 1970. I am currently using JavaScript's date utility, which automatically performs the conversion of the date based on the timezone of the location where it was called from.
I haven't done this bit yet, but shouldn't be too difficult to implement. |
At this point, I'm basically writing a table formatter, but replacing the tab character with a space. Maybe, instead of doing that, I can simply modify the |
Yes I'm just changing my tune here and saying that we can buffer it up and sort it.
No that's not what I meant here. I'm saying long mode in the sense of the |
No that won't work, the table formatter should be using tab as tab spacing in the terminal fixes the tab width. However I noticed that You could have a different table output formatter thie time call it |
In the latest commit, I actually implemented this already. So far, nothing has broken, and everything works as intended. This is the output that the table formatter gives: [aryanj@matrix-34xx]$ pk secrets ls newvault:/ -lH
Type Size Links Date Modified Name
- 1 65 Aug 19 10:07 .rippling-dns-record.txt
- 1 175 Jun 13 16:53 another.txt
- 1 54 Jun 13 16:53 newsecret.txt
- 1 17 Jun 13 16:51 secret1.txt
d 2 0 Aug 19 12:21 test/
[aryanj@matrix-34xx]$ pk secrets ls newvault:/ -l
- 1 65 Aug 19 10:07 .rippling-dns-record.txt
- 1 175 Jun 13 16:53 another.txt
- 1 54 Jun 13 16:53 newsecret.txt
- 1 17 Jun 13 16:51 secret1.txt
d 2 0 Aug 19 12:21 test/ |
I have noticed two problems with how the table formatter formats the
I will add an option to the table formatter to change the text alignment from left aligned to right aligned. The second issue I had was the auto-escaping done by the table formatter. It would escape things like quotations, but not space. For example, it would render My first idea was to simply have an option to override the regex and the replacer, but that seemed too janky, so Brian helped me come up with another solution. Instead of overriding the escape function and regex, I can just disable it, and provide it with the padded strings by myself. The disabling of the escaping function will again be an option in the table formatter. I need to use the table formatter, as the output of |
This actually does not work. [aryanj@matrix-34xx]$ pk secrets ls newvault -l
- 1 65 Aug 19 10:07 .hidden-file.txt
- 1 175 Jun 13 16:53 another.txt
- 1 54 Jun 13 16:53 longer secret.txt
- 1 17 Jun 13 16:51 secret1.txt
d 2 0 Aug 19 12:21 test/ It also aligns the files to the right, not just the text like hard links, block sizes, etc. basically any field which can have a variable length. Thus, the easiest solution right now appears to create a utility function which will apply padding for any field manually, then manually padding the hard links and file size field. Or, alternatively, just ignoring this issue and keeping everything left-aligned. |
For 6., it would be slightly more complex supporting that as we would need to perform multiple RPC calls and somehow handle displaying them all. This would especially be an issue as this style would be inherently incompatible with how the table formatter works, so this would force me to basically write a custom formatter in the For 7., I don't think supporting it is a good idea. We are only displaying information for the files within the vault, and only the information which is relevant to it. The filesystem would have more information than the information we show for vault files. Moreover, I support syntax where you can just specify the vault name and all the files in the vault are automatically listed out. If we do end up implementing this functionality, then it would break this syntax. [aryanj@matrix-34xx]$ pk secrets ls newvault My suggestion is to let the user use their own system utilities to interact with their local file system. For example, scripts made in windows relying on its Brian's argument was that we would need to seamlessly interact with the user's file system for commands like |
The entire point of #32 is for it to function seamlessly like the unix commands. To do that both 6. and 7. are requirements for this. Specifying multiple paths is pretty essential to the LS command. In fact, its why it works at all with the glob expansion the shell does. But the main thing is, without using the globWalk utility to specify a globstar pattern we have not way of outputting multiple files unless we specify multiple paths. Making multiple RPC calls shouldn't be a problem here. It should be expected at this stage.
|
As per our discussion, I am removing the extended features included in this PR, including the detailed view, globbing, etc, as those features aren't high on the priority list now. |
7d44931
to
a4ed695
Compare
wip: working on adding unix-like ls support feat: added ls for listing vaults with long and short formatting [ci skip] feat: added option to show hidden files and removed redundant metadata [ci skip] feat: added directory traversal, showing hidden files by default [ci skip] chore: simplified pattern formatting code [ci skip] chore: updated long list formatting and added header option [ci skip] chore: updated long list formatting and header formatting [ci skip] chore: updated date formatter to handle old files chore: removed redundant directory rpc call and removed leading slashes from file paths [ci skip] chore: added error handling for invalid patterns [ci skip] chore: updated long list formatting using table formatter [ci skip] chore: added new parser for parsing secret filepaths [ci skip] chore: simplifying functionality for merging [ci skip] chore: cleaning up code [ci skip] chore: fixed tests chore: consolidating redundant maps fix: lint chore: updated tests and added edge cases fix: lint [ci skip] chore: updated parsers [ci skip]
bcf074b
to
c2d70c1
Compare
Brian helped me with some minor issues and cleaning up the code for the parsers. This PR now looks ready. Merging. |
Description
This PR works on implementing UNIX-like
secrets ls
experience, as if one wasls
-ing using thecoreutils
'sls
command. As described in the issue spec, not allls
commands will be supported, only some major and useful ones.Issues Fixed
secrets ls
command #245Tasks
ls
supportFinal checklist