Skip to content

[hl] use classpaths relative_path for get_relative_path #12219

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

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

yuxiaomao
Copy link
Contributor

@yuxiaomao yuxiaomao commented Apr 28, 2025

Fixes #12215

HL omits the src/ part when there is a -cp src

CPP does not remove it because there is a bug on the relativePath function used by cpp, where src/ is given as a relative path inside class_paths, but the pfile is first turn into absolute path before comparison

let strip_file ctx file = match Gctx.defined ctx Define.AbsolutePath with
| true -> Path.get_full_path file
| false -> ctx.class_paths#relative_path file

With this change, hl and cpp share the same relative path function, which ignore src/ when there is a -cp src.

Don't know where should I change for jvm though.

Use the same relative_path function as hxcpp & trace. This still guarantee relative the same std and lib path. This also fix hl's std relative path detection error (caused by an empty classpath before std).

@yuxiaomao
Copy link
Contributor Author

Well, not the best choice because trace depends on it too :')

@Simn
Copy link
Member

Simn commented Apr 28, 2025

IMO all relative paths should always be printed relative to . because that's the reference point for users...

@yuxiaomao yuxiaomao changed the title [classPaths][hl/cpp] fix relative_path on -cp src [hl] use classpaths relative_path for get_relative_path Apr 28, 2025
@yuxiaomao
Copy link
Contributor Author

But for std and lib it's not relative to . , so it's not very consistant. At the same time lib are not user's files, so it makes sens too.
trace use this too and it did not surprised me, so maybe that's ok after all x)

@Simn
Copy link
Member

Simn commented Apr 29, 2025

I'm a bit salty that I open an issue about HL's relative path printing being inferior, and then you open a PR to make other targets behave like HL. x)

Relative file paths should be relative to the current working directory because that's the reference point for both users and the IDEs they use. Trimming the relative src or whatever directory makes the file paths ambiguous and impossible to find without context. A printout of src/File.hx can be understood as ./src/File.hx which is nice and easy to locate, whereas what we have with this change is essentially ./File.hx which doesn't exist.

I really don't understand why anyone would consider this better.

@yuxiaomao
Copy link
Contributor Author

I took some time to hesitate between make HL behave like others targets / make others target behave like HL. I thought that relative file path is always relative to class paths (the case of std and lib path), that's why at some point I was trying to "fix" it.

I apologize for the confusion :/

This PR now only modify HL.

@yuxiaomao
Copy link
Contributor Author

Additional mini bugs observed:

  • relative_path does not work the same way if we use -cp <absolute/path/to>/src (trim src) instead of -cp src (not trim src).
  • Path.get_full_path on windows do not remove the trailing /

@skial skial mentioned this pull request May 1, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[hl] Call stack paths are missing leading class path
2 participants