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

Relative paths are not correct #4

Closed
nameless-ross opened this issue Jun 8, 2020 · 12 comments
Closed

Relative paths are not correct #4

nameless-ross opened this issue Jun 8, 2020 · 12 comments
Assignees

Comments

@nameless-ross
Copy link
Contributor

nameless-ross commented Jun 8, 2020

Hi,

I have tried out the tool but after requiring AmericanBritishSpellings.php in my code I'm getting fatal errors that AmericanBritishSpellings_Words.php failed to open.

On closer inspection it looks as if the relative paths are incorrect for AmericanBritishSpellings_Words.php and AmericanBritishWords_[LETTER].php.

Everything works fine if I replace the relative path ../classes/Language with __DIR__

Many thanks in advance!
Ross

@HoldOffHunger
Copy link
Owner

HoldOffHunger commented Jun 8, 2020

Hi, Ross!

Thanks for the heads up, I'll take a closer look at this tonight. Relative paths are indeed just that, relative, so it might be something that requires a code change or just improved documentation.

This project is actually an isolation of the relevant code for the GG-CMS, you can see the AmericanBritishSpellings***.php files located here for it: https://github.com/HoldOffHunger/GreenGluonCMS/tree/master/classes/Language . It's possible that you're not using a .../classes/... dir, but, maybe that should be addressed in documentation or code.

This is also where the demo runs for it, https://www.revoltlib.com/spellchecker.php . GG-CMS is designed to keep apache-hosted domain directories and code directories separate (standard security protocol).

Thanks again, I'll know more when I look closer tonight.

@HoldOffHunger HoldOffHunger self-assigned this Jun 8, 2020
@nameless-ross
Copy link
Contributor Author

Thanks for your quick response, I appreciate you looking into this.

Just a thought:

Are you perhaps setting the include path from the webserver? The path in my case is relative to the file itself (AmericanBritishSpellings.php) so it's attempting to load the file using the following path:
/[PROJECT_ROOT]/libraries/convert-british-to-american-spellings/classes/Language/../classes/Language/AmericanBritishSpellings_Words.php

The code is stored in a directory specifically for 3rd-party libraries and is being required using an absolute path.

Many thanks!
Ross

@HoldOffHunger
Copy link
Owner

HoldOffHunger commented Jun 9, 2020

Hey, again, Ross:

I spent some time thinking about this. TLDR: Your idea of using __DIR__ . was a pretty good suggestion, and I've implemented it into this project.

The DR part: The project that I isolated this from, well, it's imperfect. I have the dir setup as so...

  • /apache-hosted-domain-name.com/index.php
  • /classes/Languages/mybritishamericancode.php
  • /classes/Languages/*etc*

So, when a user hit the .htaccess and redirecting index.php of apache-hosted-domain-name.com, to include the british-american code, I needed to step up one directory, hence ../, and then into the right directory, so, /classes/Languages/..., therefore require('../classes/Languages/...*...), etc..

But your method is perfect when it comes to making this project 100%-working-out-of-the-box, especially given how our code samples have worked.

I just pushed improvements to use __DIR__ in the two PHP files, AmericanBritishSpellings.php and AmericanBritishSpellings_Words.php. I tested things relatively thoroughly, let me know if I missed anything, thanks!

Will give this ticket some time before closing it, if all is good.

@nameless-ross
Copy link
Contributor Author

nameless-ross commented Jun 9, 2020

Hi,

Thanks again for spending time on this. I'm still having trouble with the filepaths and I'm a bit confused what the differences are between our setups.

Judging from the documentation, __DIR__ is the absolute path of the current file's directory (in this example the AmericanBritishSpellings.php file). This is the way it's working at my end and this is the error:
Failed opening required '/apache-hosted-domain-name.com/web/libraries/convert-british-to-american-spellings/classes/Language/classes/Language/AmericanBritishSpellings_Words.php'

From the new commit it looks as if __DIR__ is the absolute path of the "project root" directory.

In my project I'm using Drupal so I created some test code to check the framework wasn't doing anything I wasn't aware of.

I created the following file structure:

/sites/apache-hosted-domain-name.com/convert-british-to-american-spellings/..
/sites/apache-hosted-domain-name.com/index.php

This is the contents of index.php:

<?php

require_once __DIR__ . '/convert-british-to-american-spellings/classes/Language/AmericanBritishSpellings.php';

$string = 'travelling';
$spellings = new AmericanBritishSpellings([]);

echo '<h1>Spelling test</h1><dl><dt>Original</dt><dd>' . $string . '</dd><dt>US spelling</dt><dd>';
echo $spellings->SwapBritishSpellingsForAmericanSpellings(['text' => $string]) . '</dd></dl>';

Unfortunately I still couldn't get the code to work, I have created a fork and this commit fixes the problems I'm facing:
nameless-ross@cb84b31

As I said, I'm a bit confused and not sure what I'm doing differently. If the code is working fine then I'll happily continue to use the fork.

Many thanks!
Ross

@HoldOffHunger
Copy link
Owner

Oh, I looked at your forked project. I suspect maybe you moved the code in '/classes/Language/*' to just the base directory '/', and that's why your require(__DIR__ . 'AmericanBritish.php) is working fine.

Hrm, your code without those extra directories is a bit cleaner -- I'll probably fix up this project, too, later tonight, to have the same dir structuring.

Also (NOTE TO SELF), I should probably add some installation instructions to README.MD once the above is all said and done.

Thanks again for testing, nameless-ross. That's usually above and beyond the call of duty, so I appreciate it!

@HoldOffHunger
Copy link
Owner

HoldOffHunger commented Jun 17, 2020

Hi, again, Ross,

Sorry for the delay, been busy, etc.. (have added 2,000 to 3,000 new words to the codebase per newest release)

I've moved all of the code into the lib dir, like an ordinary db. Hopefully, this is a bit better in being clearer about its operation. Let me know if you think this is enough, thanks.

https://github.com/HoldOffHunger/convert-british-to-american-spellings/tree/master/lib

New release notes:

https://github.com/HoldOffHunger/convert-british-to-american-spellings/releases/tag/1.02

@nameless-ross
Copy link
Contributor Author

Hi there,

Sorry about my late response! The new structure is an improvement however, I still have problems getting this to work out of the box.

The issue for me are the paths specified on these lines:

https://github.com/HoldOffHunger/convert-british-to-american-spellings/blob/master/lib/AmericanBritishSpellings.php#L19

https://github.com/HoldOffHunger/convert-british-to-american-spellings/blob/master/lib/AmericanBritishSpellings_Words.php#L59

In order to fix the issue I have to change the paths to a path relative to the file itself and not the project structure. The above lines in my fork have been changed to:

// AmericanBritishSpellings.php
require(__DIR__ . '/AmericanBritishSpellings_Words.php');
// AmericanBritishSpellings_Words.php
$word_directory = __DIR__ . '/Words/AmericanBritish/';

Would you mind testing those realtive paths at your end?

Many thanks,
Ross

@HoldOffHunger
Copy link
Owner

Ah, you are right, sorry about that.

https://github.com/HoldOffHunger/convert-british-to-american-spellings/blob/master/lib/AmericanBritishSpellings.php

https://github.com/HoldOffHunger/convert-british-to-american-spellings/blob/master/lib/AmericanBritishSpellings_Words.php

Lines have been updated:

require(__DIR__ . '/AmericanBritishSpellings_Words.php');

$word_directory = DIR . '/Words/AmericanBritish/';

Thanks for being patient with this!

@nameless-ross
Copy link
Contributor Author

Hi,

Sorry, once last change needed and then I think we are there!

It needs to be require_once() rather than require() on both of those lines. If the code is used more than once it attempts to redeclare those classes and I get the error:
Got error 'PHP message: PHP Fatal error: Cannot declare class AmericanBritishWords_A, because the name is already in use

Many thanks!
Ross

@HoldOffHunger
Copy link
Owner

Hey hey,

Ah, good point! Updates have been added, for both the requires in AmericanBritishSpellings.php and AmercianBritishSpellings_Words.php.

Let me know if all is good.

@nameless-ross
Copy link
Contributor Author

Hi,

Updates work great, I've created a pull request with a couple of fixes for new bugs introduced.

Many thanks,
Ross

@HoldOffHunger
Copy link
Owner

Perfect, I love features! Thanks for your patience and contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants