Skip to content
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

BUG: os.isHidden doesn't work with directories; should use just paths, not filesystem access #8225

Closed
timotheecour opened this issue Jul 6, 2018 · 10 comments

Comments

Projects
None yet
5 participants
@timotheecour
Copy link
Contributor

commented Jul 6, 2018

  • currently "/Users/timothee/.ssh".isHidden is false
    it should be true (ie, isHidden shouldn't care on whether the argument is a file or a directory)
    Current behavior is very unintuitive and causes bugs: eg the fix for citycide/glob#7 (comment) doesn't work

(haven't tried on windows but maybe it's similar).

  • currently, "foo/.unexistingpath".isHidden returns false (assuming that path doesn't exist in filesystem). It should return true IMO. on posix, isHidden shouldn't access the filesystem, but just use string manipulation. On windows, maybe that's not the case.

/cc @Araq if you agree with these I can send a PR to fix posix

EDIT

  • java: behaves same as I recommend:
// package com.javatutorialhq.java.examples;
import java.io.File;
public class test1 {
  public static void main(String[] args) {
    File file = new File(".foo");
    boolean result = file.isHidden();
    System.out.println(file + " isHidden: " + result);
  }
}
//prints: .foo isHidden: true
bool isHidden(const bf::path &p)
{
    bf::path::string_type name = p.filename();
    if(name != ".." &&
       name != "."  &&
       name[0] == '.')
    {
       return true;
    }

    return false;
}

isHidden("a/.foo") => true

@Araq

This comment has been minimized.

Copy link
Member

commented Jul 6, 2018

Heh, I don't know. Probably you're right and it should care about paths in general.

@dom96

This comment has been minimized.

Copy link
Member

commented Jul 10, 2018

As is often the case, it's good to ask this question: how do other languages handle this?

@timotheecour

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2018

@dom96 I edited my answer above to add what I could find from other languages (couldn't find much lang that support isHidden); everything I found is 100% consistent with my proposal.

Regarding whether it should apply to both files and dirs, that's just common sense.
Regarding whether it should work (on posix) on paths that don't exist, that's also common sense since only string is accessed to determine that, and that's what all answers I could find did anyway (on posix)

@citycide

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2018

@dom96 It's surprising that this isn't in more stdlibs considering it's a bit of a pain to handle across systems. Yes it's fairly easy on posix but Windows requires attribute checks. I really appreciate that this is included.

Should dots be taken into account even on Windows though? Maybe optionally? Lots of dot files are not technically hidden when they probably should be.

@timotheecour

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2018

@citycide @dom96 @Araq
how about the following:
ospaths.isDotPath => checks only for paths, works cross platform
os.isHidden => calls ospaths.isDotPath on posix, calls windows specific on windows (ie, same as my top level recommendation)

@Araq

This comment has been minimized.

Copy link
Member

commented Jul 11, 2018

Should dots be taken into account even on Windows though? Maybe optionally? Lots of dot files are not technically hidden when they probably should be.

Never ever. Our stdlib is not responsible for fixing half-assed Unix ports to Windows.

@andreaferretti

This comment has been minimized.

Copy link
Collaborator

commented Jul 11, 2018

os.isDotPath would not be enough: I am pretty sure that on Linux files that end in ~ are hidden as well.

If the OS exposes a way to figure out whether a file or directory is hidden, that should be used (what happens on windows anyway). I am not sure whether something like that exists on Linux, though

@timotheecour

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2018

@andreaferretti

os.isDotPath would not be enough: I am pretty sure that on Linux files that end in ~ are hidden as well.

just tried, doesn't seem like that's correct:

ls
total 0
-rw-r--r-- 1 timothee 0 Jul 11 02:06 foo~
-rw-r--r-- 1 timothee 0 Jul 11 02:06 foo~2
prompt:lhmbp /private/tmp/d01  0.020 $ ls -a
total 0
drwxr-xr-x  5 timothee 160 Jul 11 02:06 ./
drwxrwxrwt 28 root     896 Jul 11 02:06 ../
-rw-r--r--  1 timothee   0 Jul 11 02:06 .bar
-rw-r--r--  1 timothee   0 Jul 11 02:06 foo~
-rw-r--r--  1 timothee   0 Jul 11 02:06 foo~2

likewise on OSX.

@andreaferretti

This comment has been minimized.

Copy link
Collaborator

commented Jul 11, 2018

It seems you're right. Most file managers hide those as well, though

@dom96

This comment has been minimized.

Copy link
Member

commented Jul 11, 2018

Don't add any new procs. Just implement your proposal.

timotheecour added a commit to timotheecour/Nim that referenced this issue Jul 14, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Jul 14, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Jul 14, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Jul 14, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Jul 17, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Oct 10, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Oct 10, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Oct 12, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Oct 13, 2018

@Araq Araq closed this in #8315 Oct 14, 2018

Araq added a commit that referenced this issue Oct 14, 2018

fix #8225 os.isHidden was buggy on posix (#8315)
* fix #8225 isHidden was broken on posix

* scope rest of tos.nim under blocks to avoid variable scope bugs

krux02 added a commit to krux02/Nim that referenced this issue Oct 15, 2018

fix nim-lang#8225 os.isHidden was buggy on posix (nim-lang#8315)
* fix nim-lang#8225 isHidden was broken on posix

* scope rest of tos.nim under blocks to avoid variable scope bugs

narimiran added a commit to narimiran/Nim that referenced this issue Oct 31, 2018

fix nim-lang#8225 os.isHidden was buggy on posix (nim-lang#8315)
* fix nim-lang#8225 isHidden was broken on posix

* scope rest of tos.nim under blocks to avoid variable scope bugs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.