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

Open relative files from executing program path #123

Merged
merged 2 commits into from
Jun 28, 2022

Conversation

giancarlopernudisegura
Copy link
Contributor

When giving a relative file path, RARS will look from the path of the RISC-V program, not the from the RARS path. This is more intuitive. Absolute file paths remain unaffected.

@TheThirdOne
Copy link
Owner

As currently implemented this should break programs that execute using the API or launch from the CLI. I don't believe either sets Global.program. If Global.program is null it should default to the existing behavior. Particularly when launching from the CLI, this feature would be unintuitive.

This is more intuitive

There are situations where this could be very confusing. If you have multiple files open, are using the setting to assemble all open, and some of the files are in different directories, the selected file when you assemble could change the CWD (current working directory).

Additionally, I think most IDEs use a single CWD per workspace/project and do not change the CWD based on which file in the "main" file. For example IntelliJ's default behavior is to set the CWD to the base directory of the current project.

So while I agree that students may find this more intuitive, it would be counter what most existing software does and has the opportunity to be very confusing so I will not merge it as is.

If this were behind a setting named something like "Derive current working directory from main file" I would merge it though.

@giancarlopernudisegura
Copy link
Contributor Author

I believe I've addressed your feedback with the latest commit. There is now a setting option to enable this if the user wants it when using the GUI. When using the CLI, it uses the RARS directory as before.

@TheThirdOne TheThirdOne merged commit 8925450 into TheThirdOne:master Jun 28, 2022
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.

2 participants