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.

Sign up for GitHub

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

Jump to bottom

Try to exploit newtypes over FilePath more to better separate semantics #249

Closed
Martoon-00 opened this issue Dec 19, 2022 · 1 comment · Fixed by #263
Closed

Try to exploit newtypes over FilePath more to better separate semantics #249

Martoon-00 opened this issue Dec 19, 2022 · 1 comment · Fixed by #263
Assignees
@aeqz

Comments

@Martoon-00
Copy link
Member

Martoon-00 commented Dec 19, 2022 •
edited
Loading

Clarification and motivation

After #230 we still have some ambiguities around FilePath, e.g. it may be always in Posix format or platform dependent.

I first abandoned this idea, but it starts getting more desired to me just as a measure to make the code more readable. Namely: let's introduce some MarkdownFilePath or PosixFilePath that would be used in markdown links. And leave FilePath for platform-dependent version of path.

This way one can easily understand the expected format at any place in the code just by looking at the type.

We may end up using posix-formatted paths in at least two places after #230 - in FileInfo and in error type, so having a separate type starts to make sense for me.

On implementation notes:

  • We will likely need to start separating local links and remote links in FileInfo, let's probably do so.

Acceptance criteria

  • We try to introduce the mentioned newtype.
  • Looking at the result, we assess how much sense it makes.
@Martoon-00
Copy link
Member Author

@aeqz I'd suggest doing this after #230 but before #239.

@Martoon-00 Martoon-00 mentioned this issue Dec 21, 2022
Open
aeqz added a commit that referenced this issue Jan 9, 2023
@aeqz
[#239-#249] Further filepath refactor
65653be 
Problem: after refactoring the FilePath usages in the codebase to have a
canonical representation of them, we noticed that further improvements
could be applied, such as clarifying whether the path is system
dependent and avoiding absolute file system paths.

Solution: we now use POSIX relative paths during the analysis, and
system dependant ones for reading file contents in the scan phase.
@aeqz aeqz mentioned this issue Jan 9, 2023
Merged
8 tasks
aeqz added a commit that referenced this issue Jan 13, 2023
@aeqz
[#239][#249] Further filepath refactor
2984aea 
Problem: after refactoring the FilePath usages in the codebase to have a
canonical representation of them, we noticed that further improvements
could be applied, such as clarifying whether the path is system
dependent and avoiding absolute file system paths.

Solution: we now use POSIX relative paths during the analysis, and
system dependant ones for reading file contents in the scan phase.
aeqz added a commit that referenced this issue Jan 18, 2023
@aeqz
[#239][#249] Further filepath refactor
a7e96ab 
Problem: After refactoring the FilePath usages in the codebase to have a
canonical representation of them, we noticed that further improvements
could be applied, such as clarifying whether the path is system
dependent and avoiding absolute file system paths.

Solution: We now use POSIX relative paths during the analysis, and
system dependent ones for reading file contents in the scan phase.
aeqz added a commit that referenced this issue Jan 20, 2023
@aeqz
[#239][#249] Further filepath refactor
1dd9cb4 
Problem: After refactoring the FilePath usages in the codebase to have a
canonical representation of them, we noticed that further improvements
could be applied, such as clarifying whether the path is system
dependent and avoiding absolute file system paths.

Solution: We now use POSIX relative paths during the analysis, and
system dependent ones for reading file contents in the scan phase.
aeqz added a commit that referenced this issue Jan 24, 2023
@aeqz
[#239][#249] Parse quoted git ls-files
3507b6b 
Problem: When a file name contains unusual characters, its output line
in git ls-files is quoted and Xrefcheck parses the file name wrong.

Solution: If a git ls-files output line starts with a double quote, use
the String Read instance for parsing it.
aeqz added a commit that referenced this issue Jan 24, 2023
@aeqz
[#239][#249] Further filepath refactor
ffea622 
Problem: After refactoring the FilePath usages in the codebase to have a
canonical representation of them, we noticed that further improvements
could be applied, such as clarifying whether the path is system
dependent and avoiding absolute file system paths.

Solution: We now use POSIX relative paths during the analysis, and
system dependent ones for reading file contents in the scan phase.
aeqz added a commit that referenced this issue Jan 25, 2023
@aeqz
[#239][#249] Simplify progress bar interface
1eecd7d
aeqz added a commit that referenced this issue Jan 25, 2023
@aeqz
[#239][#249] Simplify progress bar interface
aa1818e 
Problem: After changing the progress bar interface to require progress
unit witnesses, some functions related to an anonymous task timestamp
also started to require a progress unit witness, which complicates its
usage unnecessarily.

Solution: Do not require a progress unit witness for setTaskTimestamp
and getTaskTimestamp.
aeqz added a commit that referenced this issue Jan 25, 2023
@aeqz
[#239][#249] Parse quoted git ls-files
bfa77fc 
Problem: When a file name contains unusual characters, its output line
in git ls-files is quoted and Xrefcheck parses the file name wrong.

Solution: Use the git ls-files -z option to output lines verbatim with
null character line terminators.
aeqz added a commit that referenced this issue Jan 25, 2023
@aeqz
[#239][#249] Further filepath refactor
aec4141 
Problem: After refactoring the FilePath usages in the codebase to have a
canonical representation of them, we noticed that further improvements
could be applied, such as clarifying whether the path is system
dependent and avoiding absolute file system paths.

Solution: We now use POSIX relative paths during the analysis, and
system dependent ones for reading file contents in the scan phase.
aeqz added a commit that referenced this issue Jan 25, 2023
@aeqz
[#239][#249] Simplify progress bar interface
8013bf7 
Problem: After changing the progress bar interface to require progress
unit witnesses, some functions related to an anonymous task timestamp
also started to require a progress unit witness, which complicates its
usage unnecessarily.

Solution: Do not require a progress unit witness for setTaskTimestamp
and getTaskTimestamp.
aeqz added a commit that referenced this issue Jan 27, 2023
@aeqz
[#239][#249] Further filepath refactor
eea6118 
Problem: After refactoring the FilePath usages in the codebase to have a
canonical representation of them, we noticed that further improvements
could be applied, such as clarifying whether the path is system
dependent and avoiding absolute file system paths.

Solution: We now use POSIX relative paths during the analysis, and
system dependent ones for reading file contents in the scan phase.
aeqz added a commit that referenced this issue Jan 27, 2023
@aeqz
[#239][#249] Simplify progress bar interface
71589e5 
Problem: After changing the progress bar interface to require progress
unit witnesses, some functions related to an anonymous task timestamp
also started to require a progress unit witness, which complicates its
usage unnecessarily.

Solution: Do not require a progress unit witness for getTaskTimestamp.
aeqz added a commit that referenced this issue Jan 27, 2023
@aeqz
[#239][#249] Parse quoted git ls-files
f16f95c 
Problem: When a file name contains unusual characters, its output line
in git ls-files is quoted and Xrefcheck parses the file name wrong.

Solution: Use the git ls-files -z option to output lines verbatim with
null character line terminators. The file not found message has also
been improved for the case in which its link contains a backslash.
aeqz added a commit that referenced this issue Jan 27, 2023
@aeqz
[#239][#249] Add path case sensitivity tests
236a006 
Problem: We currenlty have golden tests for anchor case-sensitivity, but
not for path case-sensitivity.

Solution: A golden test for path case-sensitivity has been added and the
previous one has been renamed.
@aeqz aeqz closed this as completed in #263 Jan 27, 2023
aeqz added a commit that referenced this issue Jan 27, 2023
@aeqz
Merge pull request #263 from serokell/aeqz/#239-#249-further-filepath…
25ec1a2 
…-refactor

[#239][#249] Further filepath refactor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aeqz aeqz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[#239][#249] Further filepath refactor serokell/xrefcheck
2 participants
@Martoon-00 @aeqz